public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [C PATCH] PR43651: add warning for duplicate qualifier
@ 2016-04-04 10:29 Mikhail Maltsev
  2016-04-07 21:50 ` Joseph Myers
  2016-04-08 17:55 ` Martin Sebor
  0 siblings, 2 replies; 9+ messages in thread
From: Mikhail Maltsev @ 2016-04-04 10:29 UTC (permalink / raw)
  To: gcc-patches, Joseph Myers, Marek Polacek, David Malcolm

[-- Attachment #1: Type: text/plain, Size: 1326 bytes --]

Hi all!

Currently GCC produces pedantic warning, if variable declaration (or
typedef) has duplicate qualifier, but only when compiling as C89 (not
C99 or C11).

The attached patch adds a new warning option to enable the same warning
in C99 and C11. It also checks whether qualifiers come from macro
expansion, e.g.:

#define CT2 const int
const CT2 x1;
CT2 const x2;

and does not warn in this case, but warns for, e.g.

void foo(const int const *x) { }
(because this probably meant to be "const int *const x")

The name for new option "-Wduplicate-decl-specifier" and wording was
chosen to match the same option in Clang.

Bootstrapped and regtested on x86_64-linux. OK for next stage 1?

-- 
Regards,
    Mikhail Maltsev


gcc/c/ChangeLog:

2016-04-04  Mikhail Maltsev  <maltsevm@gmail.com>

        PR c/43651
        * c-decl.c (declspecs_add_qual): Warn when
-Wduplicate-decl-specifier
        is enabled.
        * c-errors.c (pedwarn_c90): Return true if warned.
        * c-tree.h: Change return type to bool.

gcc/testsuite/ChangeLog:

2016-04-04  Mikhail Maltsev  <maltsevm@gmail.com>

        PR c/43651
        * gcc.dg/Wduplicate-decl-specifier.c: New test.


gcc/c-family/ChangeLog:

2016-04-04  Mikhail Maltsev  <maltsevm@gmail.com>

        PR c/43651
        * c.opt (Wduplicate-decl-specifier): New option.

[-- Attachment #2: pr43651.patch --]
[-- Type: text/x-patch, Size: 6012 bytes --]

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 4f86876..1650b25 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1000,6 +1000,10 @@ Wsubobject-linkage
 C++ ObjC++ Var(warn_subobject_linkage) Warning Init(1)
 Warn if a class type has a base or a field whose type uses the anonymous namespace or depends on a type with no linkage.
 
+Wduplicate-decl-specifier
+C ObjC Var(warn_duplicate_decl_specifier) Warning LangEnabledBy(C ObjC,Wall)
+Warn when a variable declaration has duplicate const, volatile or restrict specifier.
+
 ansi
 C ObjC C++ ObjC++
 A synonym for -std=c89 (for C) or -std=c++98 (for C++).
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 0dd2121..9902326 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -9539,21 +9539,25 @@ declspecs_add_qual (source_location loc,
   gcc_assert (TREE_CODE (qual) == IDENTIFIER_NODE
 	      && C_IS_RESERVED_WORD (qual));
   i = C_RID_CODE (qual);
+  location_t prev_loc = 0;
   switch (i)
     {
     case RID_CONST:
       dupe = specs->const_p;
       specs->const_p = true;
+      prev_loc = specs->locations[cdw_const];
       specs->locations[cdw_const] = loc;
       break;
     case RID_VOLATILE:
       dupe = specs->volatile_p;
       specs->volatile_p = true;
+      prev_loc = specs->locations[cdw_volatile];
       specs->locations[cdw_volatile] = loc;
       break;
     case RID_RESTRICT:
       dupe = specs->restrict_p;
       specs->restrict_p = true;
+      prev_loc = specs->locations[cdw_restrict];
       specs->locations[cdw_restrict] = loc;
       break;
     case RID_ATOMIC:
@@ -9564,7 +9568,17 @@ declspecs_add_qual (source_location loc,
       gcc_unreachable ();
     }
   if (dupe)
-    pedwarn_c90 (loc, OPT_Wpedantic, "duplicate %qE", qual);
+    {
+      bool warned = pedwarn_c90 (loc, OPT_Wpedantic,
+				 "duplicate %qE declaration specifier", qual);
+      if (!warned
+	  && warn_duplicate_decl_specifier
+	  && prev_loc >= RESERVED_LOCATION_COUNT
+	  && !from_macro_expansion_at (prev_loc)
+	  && !from_macro_expansion_at (loc))
+	warning_at (loc, OPT_Wduplicate_decl_specifier,
+		    "duplicate %qE declaration specifier", qual);
+    }
   return specs;
 }
 
diff --git a/gcc/c/c-errors.c b/gcc/c/c-errors.c
index d5e78b8..af157c0 100644
--- a/gcc/c/c-errors.c
+++ b/gcc/c/c-errors.c
@@ -71,11 +71,12 @@ pedwarn_c99 (location_t location, int opt, const char *gmsgid, ...)
    ISO C99 but not supported in ISO C90, thus we explicitly don't pedwarn
    when C99 is specified.  (There is no flag_c90.)  */
 
-void
+bool
 pedwarn_c90 (location_t location, int opt, const char *gmsgid, ...)
 {
   diagnostic_info diagnostic;
   va_list ap;
+  bool warned = false;
   rich_location richloc (line_table, location);
 
   va_start (ap, gmsgid);
@@ -92,6 +93,7 @@ pedwarn_c90 (location_t location, int opt, const char *gmsgid, ...)
 			       ? DK_PEDWARN : DK_WARNING);
 	  diagnostic.option_index = opt;
 	  report_diagnostic (&diagnostic);
+	  warned = true;
 	  goto out;
 	}
     }
@@ -114,7 +116,9 @@ pedwarn_c90 (location_t location, int opt, const char *gmsgid, ...)
       diagnostic_set_info (&diagnostic, gmsgid, &ap, &richloc, DK_PEDWARN);
       diagnostic.option_index = opt;
       report_diagnostic (&diagnostic);
+      warned = true;
     }
 out:
   va_end (ap);
+  return warned;
 }
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 96ab049..ac72820 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -716,7 +716,7 @@ extern void c_bind (location_t, tree, bool);
 extern bool tag_exists_p (enum tree_code, tree);
 
 /* In c-errors.c */
-extern void pedwarn_c90 (location_t, int opt, const char *, ...)
+extern bool pedwarn_c90 (location_t, int opt, const char *, ...)
     ATTRIBUTE_GCC_DIAG(3,4);
 extern bool pedwarn_c99 (location_t, int opt, const char *, ...)
     ATTRIBUTE_GCC_DIAG(3,4);
diff --git a/gcc/testsuite/gcc.dg/Wduplicate-decl-specifier.c b/gcc/testsuite/gcc.dg/Wduplicate-decl-specifier.c
new file mode 100644
index 0000000..6b939fb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wduplicate-decl-specifier.c
@@ -0,0 +1,62 @@
+/* PR43651 */
+/* { dg-do compile } */
+/* { dg-options -Wduplicate-decl-specifier } */
+
+typedef const int CT1;
+#define CT2 const int
+typedef volatile int VT1;
+#define VT2 volatile int
+typedef char *restrict RT1;
+#define RT2 char *restrict
+
+void foo(void)
+{
+    const CT1 x1;
+    const CT2 x2;
+    CT1 const x3;
+    CT2 const x4;
+    const int const x5;         /* { dg-warning "duplicate .const." } */
+    const int *const x6;
+    volatile VT1 y1;
+    volatile VT2 y2;
+    VT1 volatile y3;
+    VT2 volatile y4;
+    volatile int volatile y5;   /* { dg-warning "duplicate .volatile." } */
+    volatile int *volatile y6;
+    RT1 restrict r1;
+    RT2 restrict r2;
+    restrict RT1 r3;
+    /* "restrict RT2" is invalid */
+    char *restrict restrict r4; /* { dg-warning "duplicate .restrict." } */
+    char *restrict *restrict r5;
+}
+
+void c1(const CT1 t) { }
+void c2(const CT2 t) { }
+void c3(CT1 const t) { }
+void c4(CT2 const t) { }
+void c5(const int const t) { }  /* { dg-warning "duplicate .const." } */
+void v1(volatile VT1 t) { }
+void v2(volatile VT2 t) { }
+void v3(VT1 volatile t) { }
+void v4(VT2 volatile t) { }
+void v5(volatile int volatile t) { } /* { dg-warning "duplicate .volatile." } */
+void r1(restrict RT1 t) { }
+void r2(RT1 restrict t) { }
+void r3(RT2 restrict t) { }
+void r4(char *restrict restrict t) { }  /* { dg-warning "duplicate .restrict." } */
+
+typedef const CT1 CCT1;
+typedef const CT2 CCT2;
+typedef CT1 const CT1C;
+typedef CT2 const CT2C;
+typedef const int const CIC;    /* { dg-warning "duplicate .const." } */
+typedef volatile VT1 VVT1;
+typedef volatile VT2 VVT2;
+typedef VT1 volatile VT1V;
+typedef VT2 volatile VT2V;
+typedef volatile int volatile VIV; /* { dg-warning "duplicate .volatile." } */
+typedef RT1 restrict RT1R;
+typedef RT2 restrict RT2R;
+typedef restrict RT1 RRT1;
+typedef int *restrict restrict IRR; /* { dg-warning "duplicate .restrict." } */

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [C PATCH] PR43651: add warning for duplicate qualifier
  2016-04-04 10:29 [C PATCH] PR43651: add warning for duplicate qualifier Mikhail Maltsev
@ 2016-04-07 21:50 ` Joseph Myers
  2016-04-08 11:42   ` Mikhail Maltsev
  2016-04-08 17:55 ` Martin Sebor
  1 sibling, 1 reply; 9+ messages in thread
From: Joseph Myers @ 2016-04-07 21:50 UTC (permalink / raw)
  To: Mikhail Maltsev; +Cc: gcc-patches, Marek Polacek, David Malcolm

New options need documenting in invoke.texi.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [C PATCH] PR43651: add warning for duplicate qualifier
  2016-04-07 21:50 ` Joseph Myers
@ 2016-04-08 11:42   ` Mikhail Maltsev
  0 siblings, 0 replies; 9+ messages in thread
From: Mikhail Maltsev @ 2016-04-08 11:42 UTC (permalink / raw)
  To: Joseph Myers; +Cc: gcc-patches, Marek Polacek, David Malcolm

[-- Attachment #1: Type: text/plain, Size: 822 bytes --]

On 04/08/2016 12:50 AM, Joseph Myers wrote:
> New options need documenting in invoke.texi.
> 
Done.

-- 
Regards,
    Mikhail Maltsev

gcc/c/ChangeLog:

2016-04-08  Mikhail Maltsev  <maltsevm@gmail.com>

	PR c/43651
	* c-decl.c (declspecs_add_qual): Warn when -Wduplicate-decl-specifier
	is enabled.
	* c-errors.c (pedwarn_c90): Return true if warned.
	* c-tree.h: Change return type to bool.

gcc/ChangeLog:

2016-04-08  Mikhail Maltsev  <maltsevm@gmail.com>

	PR c/43651
	* doc/invoke.texi (Wduplicate-decl-specifier): Document new option.

gcc/testsuite/ChangeLog:

2016-04-08  Mikhail Maltsev  <maltsevm@gmail.com>

	PR c/43651
	* gcc.dg/Wduplicate-decl-specifier.c: New test.


gcc/c-family/ChangeLog:

2016-04-08  Mikhail Maltsev  <maltsevm@gmail.com>

	PR c/43651
	* c.opt (Wduplicate-decl-specifier): New option.


[-- Attachment #2: pr43651-v2.patch --]
[-- Type: text/x-patch, Size: 7011 bytes --]

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 4f86876..c45c714 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1000,6 +1000,10 @@ Wsubobject-linkage
 C++ ObjC++ Var(warn_subobject_linkage) Warning Init(1)
 Warn if a class type has a base or a field whose type uses the anonymous namespace or depends on a type with no linkage.
 
+Wduplicate-decl-specifier
+C ObjC Var(warn_duplicate_decl_specifier) Warning LangEnabledBy(C ObjC,Wall)
+Warn when a declaration has duplicate const, volatile or restrict specifier.
+
 ansi
 C ObjC C++ ObjC++
 A synonym for -std=c89 (for C) or -std=c++98 (for C++).
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 0dd2121..9902326 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -9539,21 +9539,25 @@ declspecs_add_qual (source_location loc,
   gcc_assert (TREE_CODE (qual) == IDENTIFIER_NODE
 	      && C_IS_RESERVED_WORD (qual));
   i = C_RID_CODE (qual);
+  location_t prev_loc = 0;
   switch (i)
     {
     case RID_CONST:
       dupe = specs->const_p;
       specs->const_p = true;
+      prev_loc = specs->locations[cdw_const];
       specs->locations[cdw_const] = loc;
       break;
     case RID_VOLATILE:
       dupe = specs->volatile_p;
       specs->volatile_p = true;
+      prev_loc = specs->locations[cdw_volatile];
       specs->locations[cdw_volatile] = loc;
       break;
     case RID_RESTRICT:
       dupe = specs->restrict_p;
       specs->restrict_p = true;
+      prev_loc = specs->locations[cdw_restrict];
       specs->locations[cdw_restrict] = loc;
       break;
     case RID_ATOMIC:
@@ -9564,7 +9568,17 @@ declspecs_add_qual (source_location loc,
       gcc_unreachable ();
     }
   if (dupe)
-    pedwarn_c90 (loc, OPT_Wpedantic, "duplicate %qE", qual);
+    {
+      bool warned = pedwarn_c90 (loc, OPT_Wpedantic,
+				 "duplicate %qE declaration specifier", qual);
+      if (!warned
+	  && warn_duplicate_decl_specifier
+	  && prev_loc >= RESERVED_LOCATION_COUNT
+	  && !from_macro_expansion_at (prev_loc)
+	  && !from_macro_expansion_at (loc))
+	warning_at (loc, OPT_Wduplicate_decl_specifier,
+		    "duplicate %qE declaration specifier", qual);
+    }
   return specs;
 }
 
diff --git a/gcc/c/c-errors.c b/gcc/c/c-errors.c
index d5e78b8..af157c0 100644
--- a/gcc/c/c-errors.c
+++ b/gcc/c/c-errors.c
@@ -71,11 +71,12 @@ pedwarn_c99 (location_t location, int opt, const char *gmsgid, ...)
    ISO C99 but not supported in ISO C90, thus we explicitly don't pedwarn
    when C99 is specified.  (There is no flag_c90.)  */
 
-void
+bool
 pedwarn_c90 (location_t location, int opt, const char *gmsgid, ...)
 {
   diagnostic_info diagnostic;
   va_list ap;
+  bool warned = false;
   rich_location richloc (line_table, location);
 
   va_start (ap, gmsgid);
@@ -92,6 +93,7 @@ pedwarn_c90 (location_t location, int opt, const char *gmsgid, ...)
 			       ? DK_PEDWARN : DK_WARNING);
 	  diagnostic.option_index = opt;
 	  report_diagnostic (&diagnostic);
+	  warned = true;
 	  goto out;
 	}
     }
@@ -114,7 +116,9 @@ pedwarn_c90 (location_t location, int opt, const char *gmsgid, ...)
       diagnostic_set_info (&diagnostic, gmsgid, &ap, &richloc, DK_PEDWARN);
       diagnostic.option_index = opt;
       report_diagnostic (&diagnostic);
+      warned = true;
     }
 out:
   va_end (ap);
+  return warned;
 }
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 96ab049..ac72820 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -716,7 +716,7 @@ extern void c_bind (location_t, tree, bool);
 extern bool tag_exists_p (enum tree_code, tree);
 
 /* In c-errors.c */
-extern void pedwarn_c90 (location_t, int opt, const char *, ...)
+extern bool pedwarn_c90 (location_t, int opt, const char *, ...)
     ATTRIBUTE_GCC_DIAG(3,4);
 extern bool pedwarn_c99 (location_t, int opt, const char *, ...)
     ATTRIBUTE_GCC_DIAG(3,4);
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index e9763d4..02fb10f 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -3538,6 +3538,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect Options}.
 -Wc++11-compat  -Wc++14-compat@gol
 -Wchar-subscripts  @gol
 -Wcomment  @gol
+-Wduplicate-decl-specifier @r{(C and Objective-C only)} @gol
 -Wenum-compare @r{(in C/ObjC; this is on by default in C++)} @gol
 -Wformat   @gol
 -Wimplicit @r{(C and Objective-C only)} @gol
@@ -3694,6 +3695,12 @@ float area(float radius)
 the compiler performs the entire computation with @code{double}
 because the floating-point literal is a @code{double}.
 
+@item -Wduplicate-decl-specifier @r{(C and Objective-C only)}
+@opindex Wduplicate-decl-specifier
+@opindex Wno-duplicate-decl-specifier
+Warn if a declaration has duplicate @code{const}, @code{volatile} or
+@code{restrict} specifier.  This warning is enabled by @option{-Wall}.
+
 @item -Wformat
 @itemx -Wformat=@var{n}
 @opindex Wformat
diff --git a/gcc/testsuite/gcc.dg/Wduplicate-decl-specifier.c b/gcc/testsuite/gcc.dg/Wduplicate-decl-specifier.c
new file mode 100644
index 0000000..6b939fb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wduplicate-decl-specifier.c
@@ -0,0 +1,62 @@
+/* PR43651 */
+/* { dg-do compile } */
+/* { dg-options -Wduplicate-decl-specifier } */
+
+typedef const int CT1;
+#define CT2 const int
+typedef volatile int VT1;
+#define VT2 volatile int
+typedef char *restrict RT1;
+#define RT2 char *restrict
+
+void foo(void)
+{
+    const CT1 x1;
+    const CT2 x2;
+    CT1 const x3;
+    CT2 const x4;
+    const int const x5;         /* { dg-warning "duplicate .const." } */
+    const int *const x6;
+    volatile VT1 y1;
+    volatile VT2 y2;
+    VT1 volatile y3;
+    VT2 volatile y4;
+    volatile int volatile y5;   /* { dg-warning "duplicate .volatile." } */
+    volatile int *volatile y6;
+    RT1 restrict r1;
+    RT2 restrict r2;
+    restrict RT1 r3;
+    /* "restrict RT2" is invalid */
+    char *restrict restrict r4; /* { dg-warning "duplicate .restrict." } */
+    char *restrict *restrict r5;
+}
+
+void c1(const CT1 t) { }
+void c2(const CT2 t) { }
+void c3(CT1 const t) { }
+void c4(CT2 const t) { }
+void c5(const int const t) { }  /* { dg-warning "duplicate .const." } */
+void v1(volatile VT1 t) { }
+void v2(volatile VT2 t) { }
+void v3(VT1 volatile t) { }
+void v4(VT2 volatile t) { }
+void v5(volatile int volatile t) { } /* { dg-warning "duplicate .volatile." } */
+void r1(restrict RT1 t) { }
+void r2(RT1 restrict t) { }
+void r3(RT2 restrict t) { }
+void r4(char *restrict restrict t) { }  /* { dg-warning "duplicate .restrict." } */
+
+typedef const CT1 CCT1;
+typedef const CT2 CCT2;
+typedef CT1 const CT1C;
+typedef CT2 const CT2C;
+typedef const int const CIC;    /* { dg-warning "duplicate .const." } */
+typedef volatile VT1 VVT1;
+typedef volatile VT2 VVT2;
+typedef VT1 volatile VT1V;
+typedef VT2 volatile VT2V;
+typedef volatile int volatile VIV; /* { dg-warning "duplicate .volatile." } */
+typedef RT1 restrict RT1R;
+typedef RT2 restrict RT2R;
+typedef restrict RT1 RRT1;
+typedef int *restrict restrict IRR; /* { dg-warning "duplicate .restrict." } */


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [C PATCH] PR43651: add warning for duplicate qualifier
  2016-04-04 10:29 [C PATCH] PR43651: add warning for duplicate qualifier Mikhail Maltsev
  2016-04-07 21:50 ` Joseph Myers
@ 2016-04-08 17:55 ` Martin Sebor
  2016-04-09 12:29   ` Mikhail Maltsev
  1 sibling, 1 reply; 9+ messages in thread
From: Martin Sebor @ 2016-04-08 17:55 UTC (permalink / raw)
  To: Mikhail Maltsev, gcc-patches, Joseph Myers, Marek Polacek, David Malcolm

On 04/04/2016 04:29 AM, Mikhail Maltsev wrote:
> Hi all!
>
> Currently GCC produces pedantic warning, if variable declaration (or
> typedef) has duplicate qualifier, but only when compiling as C89 (not
> C99 or C11).

Presumably that's because C89 makes duplicating a type qualifier
a constraint violation while C99 and C11 allow them.

>
> The attached patch adds a new warning option to enable the same warning
> in C99 and C11.
> It also checks whether qualifiers come from macro
> expansion, e.g.:
>
> #define CT2 const int
> const CT2 x1;
> CT2 const x2;
>
> and does not warn in this case, but warns for, e.g.
>
> void foo(const int const *x) { }
> (because this probably meant to be "const int *const x")
>
> The name for new option "-Wduplicate-decl-specifier" and wording was
> chosen to match the same option in Clang.

My version of Clang also warns in C++ mode but if I'm reading
the patch right, GCC would warn only C mode.  I would find it
surprising if GCC provided the same option as Clang but didn't
make it available in the same languages.  Do you have some
reason for leaving it out that I'm not thinking of?

Also, in C11 mode, Clang issues the warning for duplicated
_Atomic qualifiers but it doesn't look like GCC would with
the patch.  Here again, unless there's some reason not to,
I would expect GCC to issue the same warning as Clang for
the same code.

Martin

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [C PATCH] PR43651: add warning for duplicate qualifier
  2016-04-08 17:55 ` Martin Sebor
@ 2016-04-09 12:29   ` Mikhail Maltsev
  2016-04-10 20:12     ` Martin Sebor
  2016-05-10 20:10     ` [C PATCH] " Joseph Myers
  0 siblings, 2 replies; 9+ messages in thread
From: Mikhail Maltsev @ 2016-04-09 12:29 UTC (permalink / raw)
  To: Martin Sebor, gcc-patches, Joseph Myers, Marek Polacek, David Malcolm

[-- Attachment #1: Type: text/plain, Size: 1745 bytes --]

On 04/08/2016 08:54 PM, Martin Sebor wrote:
>> The name for new option "-Wduplicate-decl-specifier" and wording was
>> chosen to match the same option in Clang.
> 
> My version of Clang also warns in C++ mode but if I'm reading
> the patch right, GCC would warn only C mode.  I would find it
> surprising if GCC provided the same option as Clang but didn't
> make it available in the same languages.  Do you have some
> reason for leaving it out that I'm not thinking of?
It is an error in C++ mode. Do we want to change this behavior?

> 
> Also, in C11 mode, Clang issues the warning for duplicated
> _Atomic qualifiers but it doesn't look like GCC would with
> the patch.  Here again, unless there's some reason not to,
> I would expect GCC to issue the same warning as Clang for
> the same code.
> 
Fixed.

-- 
Regards,
    Mikhail Maltsev

gcc/c/ChangeLog:

2016-04-08  Mikhail Maltsev  <maltsevm@gmail.com>

        PR c/43651
        * c-decl.c (declspecs_add_qual): Warn when -Wduplicate-decl-specifier
        is enabled.
        * c-errors.c (pedwarn_c90): Return true if warned.
        * c-tree.h (pedwarn_c90): Change return type to bool.
        (enum c_declspec_word): Add new enumerator cdw_atomic.

gcc/ChangeLog:

2016-04-08  Mikhail Maltsev  <maltsevm@gmail.com>

        PR c/43651
        * doc/invoke.texi (Wduplicate-decl-specifier): Document new option.

gcc/testsuite/ChangeLog:

2016-04-08  Mikhail Maltsev  <maltsevm@gmail.com>

        PR c/43651
        * gcc.dg/Wduplicate-decl-specifier-c11.c: New test.
        * gcc.dg/Wduplicate-decl-specifier.c: Likewise.


gcc/c-family/ChangeLog:

2016-04-08  Mikhail Maltsev  <maltsevm@gmail.com>

        PR c/43651
        * c.opt (Wduplicate-decl-specifier): New option.

[-- Attachment #2: pr43651-v3.patch --]
[-- Type: text/x-patch, Size: 8223 bytes --]

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 4f86876..f7f844d 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1000,6 +1000,10 @@ Wsubobject-linkage
 C++ ObjC++ Var(warn_subobject_linkage) Warning Init(1)
 Warn if a class type has a base or a field whose type uses the anonymous namespace or depends on a type with no linkage.
 
+Wduplicate-decl-specifier
+C ObjC Var(warn_duplicate_decl_specifier) Warning LangEnabledBy(C ObjC,Wall)
+Warn when a declaration has duplicate const, volatile, restrict or _Atomic specifier.
+
 ansi
 C ObjC C++ ObjC++
 A synonym for -std=c89 (for C) or -std=c++98 (for C++).
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 0dd2121..bc3af02 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -9539,32 +9539,48 @@ declspecs_add_qual (source_location loc,
   gcc_assert (TREE_CODE (qual) == IDENTIFIER_NODE
 	      && C_IS_RESERVED_WORD (qual));
   i = C_RID_CODE (qual);
+  location_t prev_loc = 0;
   switch (i)
     {
     case RID_CONST:
       dupe = specs->const_p;
       specs->const_p = true;
+      prev_loc = specs->locations[cdw_const];
       specs->locations[cdw_const] = loc;
       break;
     case RID_VOLATILE:
       dupe = specs->volatile_p;
       specs->volatile_p = true;
+      prev_loc = specs->locations[cdw_volatile];
       specs->locations[cdw_volatile] = loc;
       break;
     case RID_RESTRICT:
       dupe = specs->restrict_p;
       specs->restrict_p = true;
+      prev_loc = specs->locations[cdw_restrict];
       specs->locations[cdw_restrict] = loc;
       break;
     case RID_ATOMIC:
       dupe = specs->atomic_p;
       specs->atomic_p = true;
+      prev_loc = specs->locations[cdw_atomic];
+      specs->locations[cdw_atomic] = loc;
       break;
     default:
       gcc_unreachable ();
     }
   if (dupe)
-    pedwarn_c90 (loc, OPT_Wpedantic, "duplicate %qE", qual);
+    {
+      bool warned = pedwarn_c90 (loc, OPT_Wpedantic,
+				 "duplicate %qE declaration specifier", qual);
+      if (!warned
+	  && warn_duplicate_decl_specifier
+	  && prev_loc >= RESERVED_LOCATION_COUNT
+	  && !from_macro_expansion_at (prev_loc)
+	  && !from_macro_expansion_at (loc))
+	warning_at (loc, OPT_Wduplicate_decl_specifier,
+		    "duplicate %qE declaration specifier", qual);
+    }
   return specs;
 }
 
diff --git a/gcc/c/c-errors.c b/gcc/c/c-errors.c
index d5e78b8..af157c0 100644
--- a/gcc/c/c-errors.c
+++ b/gcc/c/c-errors.c
@@ -71,11 +71,12 @@ pedwarn_c99 (location_t location, int opt, const char *gmsgid, ...)
    ISO C99 but not supported in ISO C90, thus we explicitly don't pedwarn
    when C99 is specified.  (There is no flag_c90.)  */
 
-void
+bool
 pedwarn_c90 (location_t location, int opt, const char *gmsgid, ...)
 {
   diagnostic_info diagnostic;
   va_list ap;
+  bool warned = false;
   rich_location richloc (line_table, location);
 
   va_start (ap, gmsgid);
@@ -92,6 +93,7 @@ pedwarn_c90 (location_t location, int opt, const char *gmsgid, ...)
 			       ? DK_PEDWARN : DK_WARNING);
 	  diagnostic.option_index = opt;
 	  report_diagnostic (&diagnostic);
+	  warned = true;
 	  goto out;
 	}
     }
@@ -114,7 +116,9 @@ pedwarn_c90 (location_t location, int opt, const char *gmsgid, ...)
       diagnostic_set_info (&diagnostic, gmsgid, &ap, &richloc, DK_PEDWARN);
       diagnostic.option_index = opt;
       report_diagnostic (&diagnostic);
+      warned = true;
     }
 out:
   va_end (ap);
+  return warned;
 }
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 96ab049..a3fad13 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -253,6 +253,7 @@ enum c_declspec_word {
   cdw_const,
   cdw_volatile,
   cdw_restrict,
+  cdw_atomic,
   cdw_saturating,
   cdw_alignas,
   cdw_address_space,
@@ -716,7 +717,7 @@ extern void c_bind (location_t, tree, bool);
 extern bool tag_exists_p (enum tree_code, tree);
 
 /* In c-errors.c */
-extern void pedwarn_c90 (location_t, int opt, const char *, ...)
+extern bool pedwarn_c90 (location_t, int opt, const char *, ...)
     ATTRIBUTE_GCC_DIAG(3,4);
 extern bool pedwarn_c99 (location_t, int opt, const char *, ...)
     ATTRIBUTE_GCC_DIAG(3,4);
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index e9763d4..63f21e9 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -3538,6 +3538,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect Options}.
 -Wc++11-compat  -Wc++14-compat@gol
 -Wchar-subscripts  @gol
 -Wcomment  @gol
+-Wduplicate-decl-specifier @r{(C and Objective-C only)} @gol
 -Wenum-compare @r{(in C/ObjC; this is on by default in C++)} @gol
 -Wformat   @gol
 -Wimplicit @r{(C and Objective-C only)} @gol
@@ -3694,6 +3695,13 @@ float area(float radius)
 the compiler performs the entire computation with @code{double}
 because the floating-point literal is a @code{double}.
 
+@item -Wduplicate-decl-specifier @r{(C and Objective-C only)}
+@opindex Wduplicate-decl-specifier
+@opindex Wno-duplicate-decl-specifier
+Warn if a declaration has duplicate @code{const}, @code{volatile},
+@code{restrict} or @code{_Atomic} specifier.  This warning is enabled by
+@option{-Wall}.
+
 @item -Wformat
 @itemx -Wformat=@var{n}
 @opindex Wformat
diff --git a/gcc/testsuite/gcc.dg/Wduplicate-decl-specifier-c11.c b/gcc/testsuite/gcc.dg/Wduplicate-decl-specifier-c11.c
new file mode 100644
index 0000000..c2db525
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wduplicate-decl-specifier-c11.c
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-options "-std=c11 -Wduplicate-decl-specifier" } */
+
+typedef _Atomic int AT1;
+#define AT2 _Atomic int
+
+void
+foo (void)
+{
+  _Atomic AT1 x1;
+  _Atomic AT2 x2;
+  AT1 _Atomic x3;
+  AT2 _Atomic x4;
+  _Atomic int _Atomic x5; /* { dg-warning "duplicate ._Atomic." } */
+}
+
+void a1(_Atomic AT1 t) { }
+void a2(_Atomic AT2 t) { }
+void a3(AT1 _Atomic t) { }
+void a4(AT2 _Atomic t) { }
+void a5(_Atomic int _Atomic t) { }  /* { dg-warning "duplicate ._Atomic." } */
+
+typedef _Atomic AT1 AAT1;
+typedef _Atomic AT2 AAT2;
+typedef AT1 _Atomic AT1A;
+typedef AT2 _Atomic AT2A;
+typedef _Atomic int _Atomic AIA;    /* { dg-warning "duplicate ._Atomic." } */
diff --git a/gcc/testsuite/gcc.dg/Wduplicate-decl-specifier.c b/gcc/testsuite/gcc.dg/Wduplicate-decl-specifier.c
new file mode 100644
index 0000000..d7a3919
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wduplicate-decl-specifier.c
@@ -0,0 +1,63 @@
+/* PR43651 */
+/* { dg-do compile } */
+/* { dg-options "-Wduplicate-decl-specifier" } */
+
+typedef const int CT1;
+#define CT2 const int
+typedef volatile int VT1;
+#define VT2 volatile int
+typedef char *restrict RT1;
+#define RT2 char *restrict
+
+void
+foo (void)
+{
+  const CT1 x1;
+  const CT2 x2;
+  CT1 const x3;
+  CT2 const x4;
+  const int const x5; /* { dg-warning "duplicate .const." } */
+  const int *const x6;
+  volatile VT1 y1;
+  volatile VT2 y2;
+  VT1 volatile y3;
+  VT2 volatile y4;
+  volatile int volatile y5; /* { dg-warning "duplicate .volatile." } */
+  volatile int *volatile y6;
+  RT1 restrict r1;
+  RT2 restrict r2;
+  restrict RT1 r3;
+  /* "restrict RT2" is invalid */
+  char *restrict restrict r4; /* { dg-warning "duplicate .restrict." } */
+  char *restrict *restrict r5;
+}
+
+void c1(const CT1 t) { }
+void c2(const CT2 t) { }
+void c3(CT1 const t) { }
+void c4(CT2 const t) { }
+void c5(const int const t) { }  /* { dg-warning "duplicate .const." } */
+void v1(volatile VT1 t) { }
+void v2(volatile VT2 t) { }
+void v3(VT1 volatile t) { }
+void v4(VT2 volatile t) { }
+void v5(volatile int volatile t) { } /* { dg-warning "duplicate .volatile." } */
+void r1(restrict RT1 t) { }
+void r2(RT1 restrict t) { }
+void r3(RT2 restrict t) { }
+void r4(char *restrict restrict t) { }  /* { dg-warning "duplicate .restrict." } */
+
+typedef const CT1 CCT1;
+typedef const CT2 CCT2;
+typedef CT1 const CT1C;
+typedef CT2 const CT2C;
+typedef const int const CIC;    /* { dg-warning "duplicate .const." } */
+typedef volatile VT1 VVT1;
+typedef volatile VT2 VVT2;
+typedef VT1 volatile VT1V;
+typedef VT2 volatile VT2V;
+typedef volatile int volatile VIV; /* { dg-warning "duplicate .volatile." } */
+typedef RT1 restrict RT1R;
+typedef RT2 restrict RT2R;
+typedef restrict RT1 RRT1;
+typedef int *restrict restrict IRR; /* { dg-warning "duplicate .restrict." } */

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [C PATCH] PR43651: add warning for duplicate qualifier
  2016-04-09 12:29   ` Mikhail Maltsev
@ 2016-04-10 20:12     ` Martin Sebor
  2016-04-28 12:00       ` [C PATCH PING] " Mikhail Maltsev
  2016-05-10 20:10     ` [C PATCH] " Joseph Myers
  1 sibling, 1 reply; 9+ messages in thread
From: Martin Sebor @ 2016-04-10 20:12 UTC (permalink / raw)
  To: Mikhail Maltsev, gcc-patches, Joseph Myers, Marek Polacek, David Malcolm

On 04/09/2016 06:28 AM, Mikhail Maltsev wrote:
> On 04/08/2016 08:54 PM, Martin Sebor wrote:
>>> The name for new option "-Wduplicate-decl-specifier" and wording was
>>> chosen to match the same option in Clang.
>>
>> My version of Clang also warns in C++ mode but if I'm reading
>> the patch right, GCC would warn only C mode.  I would find it
>> surprising if GCC provided the same option as Clang but didn't
>> make it available in the same languages.  Do you have some
>> reason for leaving it out that I'm not thinking of?
> It is an error in C++ mode. Do we want to change this behavior?

You're right, G++ does give an error.  I missed it in my testing.
Unlike C11, C++ requires a diagnostic for duplicated cv-qualifiers
so by issuing a warning Clang is more permissive.  My personal
inclination would be to treat this consistently between C and C++
but whether or not to change it is something Jason would need to
weigh in on.

>> Also, in C11 mode, Clang issues the warning for duplicated
>> _Atomic qualifiers but it doesn't look like GCC would with
>> the patch.  Here again, unless there's some reason not to,
>> I would expect GCC to issue the same warning as Clang for
>> the same code.
>>
> Fixed.

Great, thank you.

Martin


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [C PATCH PING] PR43651: add warning for duplicate qualifier
  2016-04-10 20:12     ` Martin Sebor
@ 2016-04-28 12:00       ` Mikhail Maltsev
  0 siblings, 0 replies; 9+ messages in thread
From: Mikhail Maltsev @ 2016-04-28 12:00 UTC (permalink / raw)
  To: Martin Sebor, gcc-patches, Joseph Myers, Marek Polacek,
	David Malcolm, Jason Merrill

On 04/10/2016 11:12 PM, Martin Sebor wrote:
> On 04/09/2016 06:28 AM, Mikhail Maltsev wrote:
>> On 04/08/2016 08:54 PM, Martin Sebor wrote:
>>>> The name for new option "-Wduplicate-decl-specifier" and wording was
>>>> chosen to match the same option in Clang.
>>>
>>> My version of Clang also warns in C++ mode but if I'm reading
>>> the patch right, GCC would warn only C mode.  I would find it
>>> surprising if GCC provided the same option as Clang but didn't
>>> make it available in the same languages.  Do you have some
>>> reason for leaving it out that I'm not thinking of?
>> It is an error in C++ mode. Do we want to change this behavior?
> 
> You're right, G++ does give an error.  I missed it in my testing.
> Unlike C11, C++ requires a diagnostic for duplicated cv-qualifiers
> so by issuing a warning Clang is more permissive.  My personal
> inclination would be to treat this consistently between C and C++
> but whether or not to change it is something Jason would need to
> weigh in on.

Ping.

-- 
Regards,
    Mikhail Maltsev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [C PATCH] PR43651: add warning for duplicate qualifier
  2016-04-09 12:29   ` Mikhail Maltsev
  2016-04-10 20:12     ` Martin Sebor
@ 2016-05-10 20:10     ` Joseph Myers
  2016-05-11 20:24       ` Mikhail Maltsev
  1 sibling, 1 reply; 9+ messages in thread
From: Joseph Myers @ 2016-05-10 20:10 UTC (permalink / raw)
  To: Mikhail Maltsev; +Cc: Martin Sebor, gcc-patches, Marek Polacek, David Malcolm

On Sat, 9 Apr 2016, Mikhail Maltsev wrote:

> gcc/c/ChangeLog:
> 
> 2016-04-08  Mikhail Maltsev  <maltsevm@gmail.com>
> 
>         PR c/43651
>         * c-decl.c (declspecs_add_qual): Warn when -Wduplicate-decl-specifier
>         is enabled.
>         * c-errors.c (pedwarn_c90): Return true if warned.
>         * c-tree.h (pedwarn_c90): Change return type to bool.
>         (enum c_declspec_word): Add new enumerator cdw_atomic.
> 
> gcc/ChangeLog:
> 
> 2016-04-08  Mikhail Maltsev  <maltsevm@gmail.com>
> 
>         PR c/43651
>         * doc/invoke.texi (Wduplicate-decl-specifier): Document new option.
> 
> gcc/testsuite/ChangeLog:
> 
> 2016-04-08  Mikhail Maltsev  <maltsevm@gmail.com>
> 
>         PR c/43651
>         * gcc.dg/Wduplicate-decl-specifier-c11.c: New test.
>         * gcc.dg/Wduplicate-decl-specifier.c: Likewise.
> 
> 
> gcc/c-family/ChangeLog:
> 
> 2016-04-08  Mikhail Maltsev  <maltsevm@gmail.com>
> 
>         PR c/43651
>         * c.opt (Wduplicate-decl-specifier): New option.

OK.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [C PATCH] PR43651: add warning for duplicate qualifier
  2016-05-10 20:10     ` [C PATCH] " Joseph Myers
@ 2016-05-11 20:24       ` Mikhail Maltsev
  0 siblings, 0 replies; 9+ messages in thread
From: Mikhail Maltsev @ 2016-05-11 20:24 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Martin Sebor, gcc-patches, Marek Polacek, David Malcolm

On 05/10/2016 10:51 PM, Joseph Myers wrote:
> On Sat, 9 Apr 2016, Mikhail Maltsev wrote:
> 
>> gcc/c/ChangeLog:
>>
>> 2016-04-08  Mikhail Maltsev  <maltsevm@gmail.com>
>>
>>         PR c/43651
>>         * c-decl.c (declspecs_add_qual): Warn when -Wduplicate-decl-specifier
>>         is enabled.
>>         * c-errors.c (pedwarn_c90): Return true if warned.
>>         * c-tree.h (pedwarn_c90): Change return type to bool.
>>         (enum c_declspec_word): Add new enumerator cdw_atomic.
>>
>> gcc/ChangeLog:
>>
>> 2016-04-08  Mikhail Maltsev  <maltsevm@gmail.com>
>>
>>         PR c/43651
>>         * doc/invoke.texi (Wduplicate-decl-specifier): Document new option.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2016-04-08  Mikhail Maltsev  <maltsevm@gmail.com>
>>
>>         PR c/43651
>>         * gcc.dg/Wduplicate-decl-specifier-c11.c: New test.
>>         * gcc.dg/Wduplicate-decl-specifier.c: Likewise.
>>
>>
>> gcc/c-family/ChangeLog:
>>
>> 2016-04-08  Mikhail Maltsev  <maltsevm@gmail.com>
>>
>>         PR c/43651
>>         * c.opt (Wduplicate-decl-specifier): New option.
> 
> OK.
> 

Committed as r236142.

-- 
Regards,
    Mikhail Maltsev

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-05-11 20:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-04 10:29 [C PATCH] PR43651: add warning for duplicate qualifier Mikhail Maltsev
2016-04-07 21:50 ` Joseph Myers
2016-04-08 11:42   ` Mikhail Maltsev
2016-04-08 17:55 ` Martin Sebor
2016-04-09 12:29   ` Mikhail Maltsev
2016-04-10 20:12     ` Martin Sebor
2016-04-28 12:00       ` [C PATCH PING] " Mikhail Maltsev
2016-05-10 20:10     ` [C PATCH] " Joseph Myers
2016-05-11 20:24       ` Mikhail Maltsev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).