[PATCH] fix warning about duplicate 'const'

[PATCH] fix warning about duplicate 'const'

[Thomas Schlichter]

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

Hi,

attached is a patch which fixes following wanings:

drivers/ide/ide-tape.c: In function `idetape_setup':
drivers/ide/ide-tape.c:4701: Warnung: duplicate `const'
drivers/video/matrox/matroxfb_g450.c: In function `g450_compute_bwlevel':
drivers/video/matrox/matroxfb_g450.c:129: Warnung: duplicate `const'
drivers/video/matrox/matroxfb_g450.c:130: Warnung: duplicate `const'
drivers/video/matrox/matroxfb_maven.c: In function `maven_compute_bwlevel':
drivers/video/matrox/matroxfb_maven.c:347: Warnung: duplicate `const'
drivers/video/matrox/matroxfb_maven.c:348: Warnung: duplicate `const'

This is done by removing the 'const' from the temporary variables of the min() 
and max() macros. For me it seems to have no negative impact, so please 
consider applying...

Best regards
   Thomas Schlichter

[-- Attachment #2: fix-duplicate-const-warning.diff --]
[-- Type: text/x-diff, Size: 678 bytes --]

--- linux-2.6.4-rc2/include/linux/kernel.h.orig	2004-03-04 07:16:34.000000000 +0100
+++ linux-2.6.4-rc2/include/linux/kernel.h	2004-03-09 00:34:21.980935992 +0100
@@ -168,15 +168,15 @@ extern void dump_stack(void);
  * "unnecessary" pointer comparison.
  */
 #define min(x,y) ({ \
-	const typeof(x) _x = (x);	\
-	const typeof(y) _y = (y);	\
-	(void) (&_x == &_y);		\
+	typeof(x) _x = (x);	\
+	typeof(y) _y = (y);	\
+	(void) (&_x == &_y);	\
 	_x < _y ? _x : _y; })
 
 #define max(x,y) ({ \
-	const typeof(x) _x = (x);	\
-	const typeof(y) _y = (y);	\
-	(void) (&_x == &_y);		\
+	typeof(x) _x = (x);	\
+	typeof(y) _y = (y);	\
+	(void) (&_x == &_y);	\
 	_x > _y ? _x : _y; })
 
 /*

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

[Andrew Morton]

Thomas Schlichter <thomas.schlichter@web.de> wrote:
>
> Hi,
> 
> attached is a patch which fixes following wanings:
> 
> drivers/ide/ide-tape.c: In function `idetape_setup':
> drivers/ide/ide-tape.c:4701: Warnung: duplicate `const'
> drivers/video/matrox/matroxfb_g450.c: In function `g450_compute_bwlevel':
> drivers/video/matrox/matroxfb_g450.c:129: Warnung: duplicate `const'
> drivers/video/matrox/matroxfb_g450.c:130: Warnung: duplicate `const'
> drivers/video/matrox/matroxfb_maven.c: In function `maven_compute_bwlevel':
> drivers/video/matrox/matroxfb_maven.c:347: Warnung: duplicate `const'
> drivers/video/matrox/matroxfb_maven.c:348: Warnung: duplicate `const'
> 
> This is done by removing the 'const' from the temporary variables of the min() 
> and max() macros. For me it seems to have no negative impact, so please 
> consider applying...

I think there was a reason for those consts in kernel.h's min() and max()
macros, but memory fails me.  Linus, do you recall?

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

[Linus Torvalds]

On Mon, 8 Mar 2004, Andrew Morton wrote:
> > 
> > This is done by removing the 'const' from the temporary variables of the min() 
> > and max() macros. For me it seems to have no negative impact, so please 
> > consider applying...
> 
> I think there was a reason for those consts in kernel.h's min() and max()
> macros, but memory fails me.  Linus, do you recall?

I'm not 100% sure, but I think they are required to make a few other 
warnings go away. The fact that gcc complains about "const typeof" is a 
gcc misfeature, I think, and should be fixed there.

The warnings the extra "const" fixes is something like:

	int a;
	const int b;

	min(a,b)

where otherwise it would complain about pointers to different types when
comparing the type of the pointer. Or something.

		Linus

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

[Thomas Schlichter]

Am Dienstag, 9. März 2004 01:32 schrieb Linus Torvalds:

~~ snip ~~

> The warnings the extra "const" fixes is something like:
>
> 	int a;
> 	const int b;
>
> 	min(a,b)
>
> where otherwise it would complain about pointers to different types when
> comparing the type of the pointer. Or something.

OK, I tested it and gcc 3.3.1 does not complain about this. So with my patch, 
the duplicate 'const' warning goes away here and no other warning occours...

But I don't know how other versions of gcc behave...

   Thomas

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

[Linus Torvalds]

On Tue, 9 Mar 2004, Thomas Schlichter wrote:
>
> Am Dienstag, 9. März 2004 01:32 schrieb Linus Torvalds:
> 
> ~~ snip ~~
> 
> > The warnings the extra "const" fixes is something like:
> >
> > 	int a;
> > 	const int b;
> >
> > 	min(a,b)
> >
> > where otherwise it would complain about pointers to different types when
> > comparing the type of the pointer. Or something.
> 
> OK, I tested it and gcc 3.3.1 does not complain about this. So with my patch, 
> the duplicate 'const' warning goes away here and no other warning occours...

Yeah, but do keep in mind that "something like" comment. I'm by no means 
sure that I remembered the exact reason correctly, and maybe they aren't 
really needed.

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.

All that code was from early 2002 (around 2.4.9), so maybe somebody can 
find the full discussion on the linux-kernel archives from January 2002 or 
so?

			Linus

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

[Linus Torvalds]

On Mon, 8 Mar 2004, Linus Torvalds wrote:
> 
> All that code was from early 2002 (around 2.4.9), so maybe somebody can 
> find the full discussion on the linux-kernel archives from January 2002 or 
> so?

Duh. Make that August/September 2001 or something. My BK dates are off for 
the early 2.4.x series, since I imported those as patches in early 2002.

		Linus

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

[Keith Duthie]

On Mon, 8 Mar 2004, Linus Torvalds wrote:

> All that code was from early 2002 (around 2.4.9), so maybe somebody can
> find the full discussion on the linux-kernel archives from January 2002 or
> so?

The thread in which the "const typeof" was introduced appears to be
"[IDEA+RFC] Possible solution for min()/max() war" from August/September
2001, and const typeof appears to have been introduced by Peter Breuer.

The arguments appear to largely be about signedness at that point. Some
mention is made of how bad -Wsign-compare is.
-- 
Just because it isn't nice doesn't make it any less a miracle.
     http://users.albatross.co.nz/~psycho/     O-   -><-

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'

[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'

[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...