Re: [PATCH] fix warning about duplicate 'const'

Re: [PATCH] fix warning about duplicate 'const'

[Richard Henderson]

On Mon, Mar 08, 2004 at 05:32:11PM -0800, Linus Torvalds wrote:
> Also, I'm not convinced this isn't a gcc regression. It would be stupid to 
> "fix" something that makes old gcc's complain, when they may be doing the 
> right thing.

Problem is, that we're supposed to complain for

	const const int x;
and
	typedef const int t;
	const t x;

The proposition that we're not supposed to complain for

	const int a;
	const __typeof(a) x;

seems dicey at best.  I'm not sure what to do about this, actually.
We might could do something with a new __nonqual_typeof(a) that
strips outermost type qualifications, but I havn't given that much
thought.


r~

Re: [PATCH] fix warning about duplicate 'const'

[Neil Booth]

Richard Henderson wrote:-

> On Mon, Mar 08, 2004 at 05:32:11PM -0800, Linus Torvalds wrote:
> > Also, I'm not convinced this isn't a gcc regression. It would be stupid to 
> > "fix" something that makes old gcc's complain, when they may be doing the 
> > right thing.
> 
> Problem is, that we're supposed to complain for
> 
> 	const const int x;
> and
> 	typedef const int t;
> 	const t x;
> 
> The proposition that we're not supposed to complain for
> 
> 	const int a;
> 	const __typeof(a) x;
> 
> seems dicey at best.  I'm not sure what to do about this, actually.
> We might could do something with a new __nonqual_typeof(a) that
> strips outermost type qualifications, but I havn't given that much
> thought.

Or you could compile in C99 mode?

Neil.

Re: [PATCH] fix warning about duplicate 'const'

[Joseph S. Myers]

On Wed, 10 Mar 2004, Neil Booth wrote:

> > seems dicey at best.  I'm not sure what to do about this, actually.
> > We might could do something with a new __nonqual_typeof(a) that
> > strips outermost type qualifications, but I havn't given that much
> > thought.
> 
> Or you could compile in C99 mode?

The gnu89-only kludge allowing compound literals in static initializers in 
certain cases, for compatibility with their old ill-defined semantics, is 
there because it was needed by Linux; I don't know if it's still needed, 
but that would prevent compiling in C99 mode where compound literals have 
only their C99 semantics as unnamed variables.

Simpler to restrict the pedwarns for duplicate qualifiers to (pedantic &&
!flag_isoc99) (in all the various cases warned for) and document this as
an extension from C99 that is accepted in C89/C90 mode.

-- 
Joseph S. Myers
jsm@polyomino.org.uk

Re: [PATCH] fix warning about duplicate 'const'

[Gabriel Dos Reis]

"Joseph S. Myers" <jsm@polyomino.org.uk> writes:

| Simpler to restrict the pedwarns for duplicate qualifiers to (pedantic &&
| !flag_isoc99) (in all the various cases warned for) and document this as
| an extension from C99 that is accepted in C89/C90 mode.

I would suggest the existing practice (in nearby language) for this:

   (1) reject "direct" duplicate cv-qualifiers, .e.g.

         const const int i = 90;       // ERROR

   (2) accept duplicates that come from typedefs, e.g.

        typedef const int T;
        const T t = 3889;               // OK

and document that as a GNU extension to C90, and compatible with C++.

-- Gaby

Re: [PATCH] fix warning about duplicate 'const'

[Richard Henderson]

On Wed, Mar 10, 2004 at 11:20:03AM +0100, Gabriel Dos Reis wrote:
> I would suggest the existing practice (in nearby language) for this:
> 
>    (1) reject "direct" duplicate cv-qualifiers, .e.g.
> 
>          const const int i = 90;       // ERROR

Perhaps you meant pedwarn, not error?

The following has been through the test suite, and doesn't warn
for *any* duplicated qualifiers.  I chose this because that's
what C99 does, and that's easier to document and more forward
compatible.


r~


	* c-decl.c (grokdeclarator): Don't warn for duplicate qualifiers
	except for pedantic c90 mode.

Index: c-decl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-decl.c,v
retrieving revision 1.483
diff -u -p -r1.483 c-decl.c
--- c-decl.c	9 Mar 2004 23:39:14 -0000	1.483
+++ c-decl.c	10 Mar 2004 22:15:28 -0000
@@ -3372,7 +3372,7 @@ grokdeclarator (tree declarator, tree de
 		{
 		  if (i == RID_CONST || i == RID_VOLATILE || i == RID_RESTRICT)
 		    {
-		      if (!flag_isoc99)
+		      if (pedantic && !flag_isoc99)
 			pedwarn ("duplicate `%s'", IDENTIFIER_POINTER (id));
 		    }
 		  else
@@ -3629,12 +3629,15 @@ grokdeclarator (tree declarator, tree de
   volatilep
     = !! (specbits & 1 << (int) RID_VOLATILE) + TYPE_VOLATILE (element_type);
   inlinep = !! (specbits & (1 << (int) RID_INLINE));
-  if (constp > 1 && ! flag_isoc99)
-    pedwarn ("duplicate `const'");
-  if (restrictp > 1 && ! flag_isoc99)
-    pedwarn ("duplicate `restrict'");
-  if (volatilep > 1 && ! flag_isoc99)
-    pedwarn ("duplicate `volatile'");
+  if (pedantic && !flag_isoc99)
+    {
+      if (constp > 1)
+	pedwarn ("duplicate `const'");
+      if (restrictp > 1)
+	pedwarn ("duplicate `restrict'");
+      if (volatilep > 1)
+	pedwarn ("duplicate `volatile'");
+    }
   if (! flag_gen_aux_info && (TYPE_QUALS (type)))
     type = TYPE_MAIN_VARIANT (type);
   type_quals = ((constp ? TYPE_QUAL_CONST : 0)
@@ -4087,12 +4090,15 @@ grokdeclarator (tree declarator, tree de
 
 	      if (erred)
 		error ("invalid type modifier within pointer declarator");
-	      if (constp > 1 && ! flag_isoc99)
-		pedwarn ("duplicate `const'");
-	      if (volatilep > 1 && ! flag_isoc99)
-		pedwarn ("duplicate `volatile'");
-	      if (restrictp > 1 && ! flag_isoc99)
-		pedwarn ("duplicate `restrict'");
+	      if (pedantic && !flag_isoc99)
+		{
+		  if (constp > 1)
+		    pedwarn ("duplicate `const'");
+		  if (volatilep > 1)
+		    pedwarn ("duplicate `volatile'");
+		  if (restrictp > 1)
+		    pedwarn ("duplicate `restrict'");
+		}
 
 	      type_quals = ((constp ? TYPE_QUAL_CONST : 0)
 			    | (restrictp ? TYPE_QUAL_RESTRICT : 0)
Index: testsuite/gcc.dg/c90-dupqual-1.c
===================================================================
RCS file: testsuite/gcc.dg/c90-dupqual-1.c
diff -N testsuite/gcc.dg/c90-dupqual-1.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gcc.dg/c90-dupqual-1.c	10 Mar 2004 22:15:29 -0000
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-std=iso9899:1990 -pedantic-errors" } */
+
+typedef const int CI;
+const const int c1;		/* { dg-error "duplicate" } */
+const CI c2;			/* { dg-error "duplicate" } */
+const CI *c3;			/* { dg-error "duplicate" } */
+
+typedef volatile int VI;
+volatile volatile int v1;	/* { dg-error "duplicate" } */
+volatile VI v2;			/* { dg-error "duplicate" } */
+volatile VI *v3;		/* { dg-error "duplicate" } */
Index: testsuite/gcc.dg/c99-dupqual-1.c
===================================================================
RCS file: testsuite/gcc.dg/c99-dupqual-1.c
diff -N testsuite/gcc.dg/c99-dupqual-1.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gcc.dg/c99-dupqual-1.c	10 Mar 2004 22:15:29 -0000
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-std=iso9899:1999 -pedantic-errors" } */
+
+typedef const int CI;
+const const int c1;		/* { dg-bogus "duplicate" } */
+const CI c2;			/* { dg-bogus "duplicate" } */
+const CI *c3;			/* { dg-bogus "duplicate" } */
+
+typedef volatile int VI;
+volatile volatile int v1;	/* { dg-bogus "duplicate" } */
+volatile VI v2;			/* { dg-bogus "duplicate" } */
+volatile VI *v3;		/* { dg-bogus "duplicate" } */
Index: testsuite/gcc.dg/gnu89-dupqual-1.c
===================================================================
RCS file: testsuite/gcc.dg/gnu89-dupqual-1.c
diff -N testsuite/gcc.dg/gnu89-dupqual-1.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gcc.dg/gnu89-dupqual-1.c	10 Mar 2004 22:15:29 -0000
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-std=gnu89 -Werror" } */
+
+typedef const int CI;
+const const int c1;		/* { dg-bogus "duplicate" } */
+const CI c2;			/* { dg-bogus "duplicate" } */
+const CI *c3;			/* { dg-bogus "duplicate" } */
+
+typedef volatile int VI;
+volatile volatile int v1;	/* { dg-bogus "duplicate" } */
+volatile VI v2;			/* { dg-bogus "duplicate" } */
+volatile VI *v3;		/* { dg-bogus "duplicate" } */

Re: [PATCH] fix warning about duplicate 'const'

[Gabriel Dos Reis]

Richard Henderson <rth@redhat.com> writes:

| On Wed, Mar 10, 2004 at 11:20:03AM +0100, Gabriel Dos Reis wrote:
| > I would suggest the existing practice (in nearby language) for this:
| > 
| >    (1) reject "direct" duplicate cv-qualifiers, .e.g.
| > 
| >          const const int i = 90;       // ERROR
| 
| Perhaps you meant pedwarn, not error?

pedwarn is OK if it makes everybody happy :-)

| The following has been through the test suite, and doesn't warn
| for *any* duplicated qualifiers.  I chose this because that's
| what C99 does, and that's easier to document and more forward
| compatible.

OK. Thanks!

-- Gaby

Re: [PATCH] fix warning about duplicate 'const'

[Gabriel Dos Reis]

Richard Henderson <rth@twiddle.net> writes:

| On Mon, Mar 08, 2004 at 05:32:11PM -0800, Linus Torvalds wrote:
| > Also, I'm not convinced this isn't a gcc regression. It would be stupid to 
| > "fix" something that makes old gcc's complain, when they may be doing the 
| > right thing.
| 
| Problem is, that we're supposed to complain for
| 
| 	const const int x;
| and
| 	typedef const int t;
| 	const t x;

If I can help with an existing pratice, in C++ the former is
invalid and the second is valid -- the extra const is just silently
ignored.  Therefore, in C++ land the construct

| 	const int a;
| 	const __typeof(a) x;

would be accepted because __typeof__ acts like an unnamed typedef[*].
(And in effect, g++ will accept the code -- assuming you abstract over
initializers).  So, it does not look like an innovation here.
I don't know whether this should be another case for "C is different
from C++".


[*] Yes, an alias that does not introduce a name is strange alias, but
    that is what __typeof__ does.

-- Gaby

Re: [PATCH] fix warning about duplicate 'const'

[Linus Torvalds]

On Tue, 9 Mar 2004, Richard Henderson wrote:
> 
> seems dicey at best.  I'm not sure what to do about this, actually.
> We might could do something with a new __nonqual_typeof(a) that
> strips outermost type qualifications, but I havn't given that much
> thought.

Ok, let's try just stripping the "const" out of the min/max macros, and
see what complains. What the code _really_ wants to do is to just compare
two types for being basically equal, and in real life what Linux really
would prefer is to have "types" as first-class citizens and being able to
compare them directly instead of playing games. And we may end up having
that as a preprocessor phase (ie I might add it to the semantic parse
thing).

		Linus

Re: [PATCH] fix warning about duplicate 'const'

[Richard Henderson]

On Wed, Mar 10, 2004 at 07:43:10AM -0800, Linus Torvalds wrote:
> Ok, let's try just stripping the "const" out of the min/max macros, and
> see what complains.

I remember what complains.  You get actual errors from

	const int x; 
	int y;
	min(x, y);


r~

Re: [PATCH] fix warning about duplicate 'const'

[Andrew Haley]

Richard Henderson writes:
 > On Wed, Mar 10, 2004 at 07:43:10AM -0800, Linus Torvalds wrote:
 > > Ok, let's try just stripping the "const" out of the min/max macros, and
 > > see what complains.
 > 
 > I remember what complains.  You get actual errors from
 > 
 > 	const int x; 
 > 	int y;
 > 	min(x, y);

Can you explain *why* we have to produce a diagnostic for "const const
int" by default?

Can't we dispatch such things to "-pedantic" ?  Or treat "const const"
like "long long" used to be, a gcc extension?

Andrew.

Re: [PATCH] fix warning about duplicate 'const'

[Paul Koning]

>>>>> "Andrew" == Andrew Haley <aph@redhat.com> writes:

 Andrew> Richard Henderson writes:
 >> On Wed, Mar 10, 2004 at 07:43:10AM -0800, Linus Torvalds wrote: >
 >> Ok, let's try just stripping the "const" out of the min/max
 >> macros, and > see what complains.
 >> 
 >> I remember what complains.  You get actual errors from
 >> 
 >> const int x; int y; min(x, y);

 Andrew> Can you explain *why* we have to produce a diagnostic for
 Andrew> "const const int" by default?

 Andrew> Can't we dispatch such things to "-pedantic" ? 

I like that suggestion.

  paul

Re: [PATCH] fix warning about duplicate 'const'

[Richard Henderson]

On Wed, Mar 10, 2004 at 06:29:43PM +0000, Andrew Haley wrote:
> Can't we dispatch such things to "-pedantic" ?  Or treat "const const"
> like "long long" used to be, a gcc extension?

Willy has just reminded me about a patch he wrote to do just this.
Looking at it.


r~

Re: [PATCH] fix warning about duplicate 'const'

[Mike Stump]

On Wednesday, March 10, 2004, at 07:43 AM, Linus Torvalds wrote:
> What the code _really_ wants to do is to just compare two types for 
> being basically equal, and in real life what Linux really would prefer 
> is to have "types" as first-class citizens and being able to compare 
> them directly instead of playing games.

:-)  Gosh, it sounds so sensible and easy.  Maybe someone will hammer 
on the std C folks for the feature...