Use more gcc extensions in the Linux headers

PATCH In review Rusty Russell 17 participants 29 messages Mar 9, 2007 – Mar 11, 2007

Share & tools

Rusty Russell

author

2007-03-09 05:57 UTC #0 lore raw

PATCH 1/1 Use more gcc extensions in the Linux headers

__builtin_types_compatible_p() has been around since gcc 2.95, and we don't use it anywhere. This patch quietly fixes that.

Signed-off-by Rusty Russell <[email protected]>

include/linux/kernel.h +3 -1

diff -r f0ff8138f993 include/linux/kernel.h
@@ -35,7 +35,9 @@ extern const char linux_proc_banner[];
 #define ALIGN(x,a)		__ALIGN_MASK(x,(typeof(x))(a)-1)
 #define __ALIGN_MASK(x,mask)	(((x)+(mask))&~(mask))
 
-#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])			      \
+	+ sizeof(typeof(int[1 - 2*!!__builtin_types_compatible_p(typeof(arr), \
+		 typeof(&arr[0]))]))*0)
 #define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f))
 #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
 #define roundup(x, y) ((((x) + ((y) - 1)) / (y)) * (y))

Stephen Rothwell

2007-03-09 06:22 UTC #1 lore raw

Quoted 3 lines · On Fri, 09 Mar 2007 16:56:32 +1100 Rusty Russell <[email protected]> wrote:

__builtin_types_compatible_p() has been around since gcc 2.95, and we don't use it anywhere. This patch quietly fixes that.

After staring at this for about 2 minutes, how about a commit message like:

Make ARRAY_SIZE complain strangely if passed a pointer instead of an array.

:-)
--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/

Linus Torvalds

2007-03-09 06:35 UTC #2 lore raw

In reply to Rusty Russell

Quoted 3 lines · Rusty Russell

__builtin_types_compatible_p() has been around since gcc 2.95, and we don't use it anywhere. This patch quietly fixes that.

Whee.

Rusty, that's a work of art.

However, I would suggest that you never show it to anybody ever again. I'm sure that in fifty years, it will be worth much more. So please keep it tightly under wraps, to keep people from gouging their eyes out^W^W^W^W^W^W^W make a killing in the art market.

Please.

Linus

Andrey Panin

2007-03-09 06:51 UTC #3 lore raw

In reply to Rusty Russell

Quoted 1 line · Rusty Russell

__builtin_types_compatible_p() has been around since gcc 2.95,

but it's not available in Intel C compiler IIRC :(

Quoted 26 lines

and we don't use it anywhere. This patch quietly fixes that.
Signed-off-by Rusty Russell <[email protected]>
diff -r f0ff8138f993 include/linux/kernel.h
include/linux/kernel.h +3 -2
@@ -35,7 +35,9 @@ extern const char linux_proc_banner[];
 #define ALIGN(x,a)		__ALIGN_MASK(x,(typeof(x))(a)-1)
 #define __ALIGN_MASK(x,mask)	(((x)+(mask))&~(mask))
 
-#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])			      \
+	+ sizeof(typeof(int[1 - 2*!!__builtin_types_compatible_p(typeof(arr), \
+		 typeof(&arr[0]))]))*0)
 #define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f))
 #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
 #define roundup(x, y) ((((x) + ((y) - 1)) / (y)) * (y))
 
 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Signature · 2 lines

Andrey Panin		| Linux and UNIX system administrator
[email protected]		| PGP key: wwwkeys.pgp.net

Christoph Hellwig

2007-03-09 07:53 UTC #4 lore raw

Quoted 4 lines · Andrey Panin

Quoted 1 line · Rusty Russell
__builtin_types_compatible_p() has been around since gcc 2.95,
but it's not available in Intel C compiler IIRC :(

So what?

Andrey Panin

2007-03-09 09:02 UTC #5 lore raw

Quoted 7 lines · Christoph Hellwig

Quoted 4 lines · Andrey Panin
Quoted 1 line · Rusty Russell
__builtin_types_compatible_p() has been around since gcc 2.95,
but it's not available in Intel C compiler IIRC :(
So what?

Kernel compilation with Intel compiler is (was ?) supported. This patch will break it.

Signature · 2 lines

Andrey Panin		| Linux and UNIX system administrator
[email protected]		| PGP key: wwwkeys.pgp.net

Christoph Hellwig

2007-03-09 09:05 UTC #6 lore raw

Quoted 2 lines · Andrey Panin

Kernel compilation with Intel compiler is (was ?) supported. This patch will break it.

It was only put in under the premise that they'll fix whatever breaks, we're not going to put any maintaince border on us to hack around broken propritary compilers.

Linus Torvalds

2007-03-09 17:48 UTC #7 lore raw

Quoted 4 lines · Christoph Hellwig

It was only put in under the premise that they'll fix whatever breaks, we're not going to put any maintaince border on us to hack around broken propritary compilers.

Well, since Rusty's macro was hoddible *anyway*, I don't think I'd apply it as-is. Breaking icc for something that ugly and not-very-important simply makes no sense.

There are better ways to do this.

For one, you could (and should!) abstract these kinds of things out, rather than put them in another macro that really does something totally different. Then, the macro could have become

	#define ARRAY_SIZE (sizeof_expression + 0*error_if_not_array)

which would already be a hell of a lot more readable. But more importantly, it's also now suddenly much easiler to abstract out for different compilers.

We *already* support different compilers through <linux/compiler.h>, and there just isn't any reason for bad code just for bad codes sake!

Linus

Trent Piepho

2007-03-10 20:36 UTC #8 lore raw

Quoted 5 lines · Linus Torvalds

On Fri, 9 Mar 2007, Christoph Hellwig wrote:
Well, since Rusty's macro was hoddible *anyway*, I don't think I'd apply it as-is. Breaking icc for something that ugly and not-very-important simply makes no sense.

Quoted 7 lines

There are better ways to do this.
For one, you could (and should!) abstract these kinds of things out, rather than put them in another macro that really does something totally different. Then, the macro could have become
	#define ARRAY_SIZE (sizeof_expression + 0*error_if_not_array)

/* Error if X is a pointer, 0 otherwise */

#define ERROR_IF_POINTER(x) \
	sizeof(int[-__builtin_types_compatible_p(typeof(x), typeof(&x[0]))])

/* Warning (div by zero) if x is a pointer, 0 otherwise */

#define WARN_IF_POINTER(x) \
	(0/!__builtin_types_compatible_p(typeof(x), typeof(&x[0])))

The gcc docs say __builtin_types_compatible_p returns 1 or 0, so the !! isn't necessary. And my gcc at least returns 0 for sizeof(int[0]).

Christoph Hellwig

2007-03-09 07:52 UTC #9 lore raw

In reply to Rusty Russell

Quoted 9 lines · Rusty Russell

__builtin_types_compatible_p() has been around since gcc 2.95, and we don't use it anywhere. This patch quietly fixes that.
Signed-off-by Rusty Russell <[email protected]>
diff -r f0ff8138f993 include/linux/kernel.h

include/linux/kernel.h extern const char linux_proc_banner[]; · line 40

Quoted 7 lines

include/linux/kernel.h +3 -1

@@ -35,7 +35,9 @@ extern const char linux_proc_banner[];
 #define ALIGN(x,a)		__ALIGN_MASK(x,(typeof(x))(a)-1)
 #define __ALIGN_MASK(x,mask)	(((x)+(mask))&~(mask))
 
-#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])			      \
+	+ sizeof(typeof(int[1 - 2*!!__builtin_types_compatible_p(typeof(arr), \
+		 typeof(&arr[0]))]))*0)

This needs a comment explaning why we're doing this, and maybe a little explanation of the combination of gcc magic and C trickery used to implement it to the brave non-uberhacker people trying to understand linux headers.

Jarek Poplawski

2007-03-09 08:31 UTC #10 lore raw

Quoted 1 line · Christoph Hellwig

On Fri, Mar 09, 2007 at 04:56:32PM +1100, Rusty Russell wrote:

...

Quoted 7 lines

include/linux/kernel.h extern const char linux_proc_banner[]; · line 40
Quoted 3 lines
include/linux/kernel.h +3 -0
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])			      \
+	+ sizeof(typeof(int[1 - 2*!!__builtin_types_compatible_p(typeof(arr), \
+		 typeof(&arr[0]))]))*0)
This needs a comment explaning why we're doing this, and maybe a little explanation of the combination of gcc magic and C trickery used to implement it to the brave non-uberhacker people trying to understand linux headers.

...and to deprive them of all the pleasure of contemplating...

Jarek P.

Theodore Ts'o

2007-03-09 11:21 UTC #11 lore raw

In reply to Rusty Russell

Quoted 4 lines · Rusty Russell

__builtin_types_compatible_p() has been around since gcc 2.95, and we don't use it anywhere. This patch quietly fixes that.
Signed-off-by Rusty Russell <[email protected]>

Is your clock set correctly? Looks like this mail was sent 23 days early. :-)

- Ted

Andi Kleen

2007-03-09 15:52 UTC #12 lore raw

In reply to Rusty Russell

Quoted 2 lines · Rusty Russell

__builtin_types_compatible_p() has been around since gcc 2.95, and we don't use it anywhere. This patch quietly fixes that.

Using BUILD_BUG_ON_ZERO() would have been somewhat cleaner.

-Andi

Rusty Russell

author

2007-03-09 22:58 UTC #13 lore raw

In reply to Rusty Russell

Quoted 2 lines · Rusty Russell

__builtin_types_compatible_p() has been around since gcc 2.95, and we don't use it anywhere. This patch quietly fixes that.

OK, many people complained that it needed a comment. Good point! ==
Add comment to ARRAY_SIZE macro.

Signed-off-by Rusty Russell <[email protected]>

include/linux/kernel.h +1 -0

diff -r 933e410f204f include/linux/kernel.h
@@ -35,6 +35,7 @@ extern const char linux_proc_banner[];
 #define ALIGN(x,a)		__ALIGN_MASK(x,(typeof(x))(a)-1)
 #define __ALIGN_MASK(x,mask)	(((x)+(mask))&~(mask))
 
+/* GCC is awesome. */
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])			      \
 	+ sizeof(typeof(int[1 - 2*!!__builtin_types_compatible_p(typeof(arr), \
 		 typeof(&arr[0]))]))*0)

Randy Dunlap

2007-03-09 23:05 UTC #14 lore raw

Quoted 3 lines · On Sat, 10 Mar 2007 09:57:32 +1100 Rusty Russell wrote:

Quoted 2 lines · Rusty Russell
__builtin_types_compatible_p() has been around since gcc 2.95, and we don't use it anywhere. This patch quietly fixes that.

Bah. Just because gcc has a "feature" doesn't mean we should use it (in this form).

Quoted 10 lines

OK, many people complained that it needed a comment. Good point! ==
Add comment to ARRAY_SIZE macro.
Signed-off-by Rusty Russell <[email protected]>
diff -r 933e410f204f include/linux/kernel.h

include/linux/kernel.h extern const char linux_proc_banner[]; · line 41

Quoted 7 lines

include/linux/kernel.h +1 -0

@@ -35,6 +35,7 @@ extern const char linux_proc_banner[];
 #define ALIGN(x,a)		__ALIGN_MASK(x,(typeof(x))(a)-1)
 #define __ALIGN_MASK(x,mask)	(((x)+(mask))&~(mask))
 
+/* GCC is awesome. */
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])			      \
 	+ sizeof(typeof(int[1 - 2*!!__builtin_types_compatible_p(typeof(arr), \
 		 typeof(&arr[0]))]))*0)

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Roland Dreier

2007-03-09 23:12 UTC #15 lore raw

In reply to Rusty Russell

Perhaps this patch can go into Wesnoth for testing for a while before we merge it into the kernel?

Robert P. J. Day

2007-03-09 23:12 UTC #16 lore raw

In reply to Rusty Russell

Quoted 14 lines · Rusty Russell

Quoted 2 lines · Rusty Russell
__builtin_types_compatible_p() has been around since gcc 2.95, and we don't use it anywhere. This patch quietly fixes that.
OK, many people complained that it needed a comment. Good point! ==
Add comment to ARRAY_SIZE macro.
Signed-off-by Rusty Russell <[email protected]>
diff -r 933e410f204f include/linux/kernel.h

include/linux/kernel.h extern const char linux_proc_banner[]; · line 41

Quoted 7 lines

include/linux/kernel.h +1 -0

@@ -35,6 +35,7 @@ extern const char linux_proc_banner[];
 #define ALIGN(x,a)		__ALIGN_MASK(x,(typeof(x))(a)-1)
 #define __ALIGN_MASK(x,mask)	(((x)+(mask))&~(mask))
 
+/* GCC is awesome. */
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])			      \
 	+ sizeof(typeof(int[1 - 2*!!__builtin_types_compatible_p(typeof(arr), \
 		 typeof(&arr[0]))]))*0)

ah, but is that "universe" kind of awesome, or the "hot dogs" kind of awesome?

http://youtube.com/watch?v=0rYT0YvQ3hs

rday

Signature · 7 lines

========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================

Jan Engelhardt

2007-03-10 01:06 UTC #17 lore raw

In reply to Rusty Russell

Quoted 14 lines · Rusty Russell

Quoted 2 lines · Rusty Russell
__builtin_types_compatible_p() has been around since gcc 2.95, and we don't use it anywhere. This patch quietly fixes that.
OK, many people complained that it needed a comment. Good point! ==
Add comment to ARRAY_SIZE macro.
Signed-off-by Rusty Russell <[email protected]>
diff -r 933e410f204f include/linux/kernel.h

include/linux/kernel.h extern const char linux_proc_banner[]; · line 41

Quoted 7 lines

include/linux/kernel.h +1 -0

@@ -35,6 +35,7 @@ extern const char linux_proc_banner[];
 #define ALIGN(x,a)		__ALIGN_MASK(x,(typeof(x))(a)-1)
 #define __ALIGN_MASK(x,mask)	(((x)+(mask))&~(mask))
 
+/* GCC is awesome. */
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])			      \
 	+ sizeof(typeof(int[1 - 2*!!__builtin_types_compatible_p(typeof(arr), \
 		 typeof(&arr[0]))]))*0)

Getting back at the macro, how would you like to have it merged?

Jan

Rusty Russell

author

2007-03-11 02:51 UTC #18 lore raw

Quoted 1 line · Jan Engelhardt

Getting back at the macro, how would you like to have it merged?

Well, this is what I sent to Linus and Andrew (many thanks to those who made appropriately whimsical *or* useful comments):

include/linux/compiler-gcc.h +3 -0

diff -r 1ccdf46b0f41 include/linux/compiler-gcc.h
@@ -22,6 +22,9 @@
     __asm__ ("" : "=r"(__ptr) : "0"(ptr));		\
     (typeof(ptr)) (__ptr + (off)); })
 
+/* &a[0] degrades to a pointer: a different type from an array */
+#define __must_be_array(a) \
+  BUILD_BUG_ON_ZERO(__builtin_types_compatible_p(typeof(a), typeof(&a[0])))
 
 #define inline		inline		__attribute__((always_inline))
 #define __inline__	__inline__	__attribute__((always_inline))

include/linux/compiler-intel.h +3 -0

diff -r 1ccdf46b0f41 include/linux/compiler-intel.h
@@ -21,4 +21,7 @@
      __ptr = (unsigned long) (ptr);				\
     (typeof(ptr)) (__ptr + (off)); })
 
+/* Intel ECC compiler doesn't support __builtin_types_compatible_p() */
+#define __must_be_array(a) 0
+
 #endif

include/linux/kernel.h +2 -1

diff -r 1ccdf46b0f41 include/linux/kernel.h
@@ -35,7 +35,8 @@ extern const char linux_proc_banner[];
 #define ALIGN(x,a)		__ALIGN_MASK(x,(typeof(x))(a)-1)
 #define __ALIGN_MASK(x,mask)	(((x)+(mask))&~(mask))
 
-#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
+
 #define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f))
 #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
 #define roundup(x, y) ((((x) + ((y) - 1)) / (y)) * (y))

Jan Engelhardt

2007-03-11 03:00 UTC #19 lore raw

Quoted 10 lines · Rusty Russell

Quoted 1 line · Jan Engelhardt
Getting back at the macro, how would you like to have it merged?
Well, this is what I sent to Linus and Andrew (many thanks to those who made appropriately whimsical *or* useful comments):
diff -r 1ccdf46b0f41 include/linux/compiler-gcc.h

include/linux/compiler-gcc.h line 27

Quoted 6 lines

include/linux/compiler-gcc.h +3 -0

@@ -22,6 +22,9 @@
    __asm__ ("" : "=r"(__ptr) : "0"(ptr));		\
    (typeof(ptr)) (__ptr + (off)); })
 
+/* &a[0] degrades to a pointer: a different type from an array */
+#define __must_be_array(a) \
+  BUILD_BUG_ON_ZERO(__builtin_types_compatible_p(typeof(a), typeof(&a[0])))

This looks _much_ nicer! (And BUILD_BUG_ON is also appropriately commented.)

Quoted 18 lines

include/linux/compiler-gcc.h +0 -0

 #define inline		inline		__attribute__((always_inline))
 #define __inline__	__inline__	__attribute__((always_inline))

diff -r 1ccdf46b0f41 include/linux/compiler-intel.h

include/linux/compiler-intel.h +3 -0

@@ -21,4 +21,7 @@
     __ptr = (unsigned long) (ptr);				\
    (typeof(ptr)) (__ptr + (off)); })
 
+/* Intel ECC compiler doesn't support __builtin_types_compatible_p() */
+#define __must_be_array(a) 0
+
 #endif

diff -r 1ccdf46b0f41 include/linux/kernel.h

include/linux/kernel.h extern const char linux_proc_banner[]; · line 42

Quoted 9 lines

include/linux/kernel.h +2 -1

@@ -35,7 +35,8 @@ extern const char linux_proc_banner[];
 #define ALIGN(x,a)		__ALIGN_MASK(x,(typeof(x))(a)-1)
 #define __ALIGN_MASK(x,mask)	(((x)+(mask))&~(mask))
 
-#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
+
 #define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f))
 #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
 #define roundup(x, y) ((((x) + ((y) - 1)) / (y)) * (y))

80 cols *cough* :)

Jan

Rusty Russell

author

2007-03-11 03:20 UTC #20 lore raw

Quoted 4 lines · Jan Engelhardt

include/linux/kernel.h extern const char linux_proc_banner[]; · line 39
Quoted 3 lines
include/linux/kernel.h +2 -1
-#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
+
80 cols *cough* :)

I think your cough added a column?

Rusty.

[email protected]

2007-03-11 21:41 UTC #21 lore raw

In reply to Rusty Russell

Quoted 2 lines · Rusty Russell

Well, this is what I sent to Linus and Andrew (many thanks to those who made appropriately whimsical *or* useful comments):

Ahh.. much better - it's now a form that even I can get my brain wrapped around :)

[email protected]

2007-03-10 04:11 UTC #22 lore raw

In reply to Rusty Russell

include/linux/kernel.h extern const char linux_proc_banner[]; · line 41

Quoted 4 lines · Rusty Russell

include/linux/kernel.h +1 -0

+/* GCC is awesome. */
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])			      \
 	+ sizeof(typeof(int[1 - 2*!!__builtin_types_compatible_p(typeof(arr), \
 		 typeof(&arr[0]))]))*0)

-/* GCC is awesome. */
+/* GCC leaves me speechless. */

Randy Dunlap

2007-03-10 04:26 UTC #23 lore raw

Review

Quoted 9 lines · On Fri, 09 Mar 2007 23:03:05 -0500 [email protected] wrote:

Quoted 4 lines · Rusty Russell

include/linux/kernel.h +1 -0

+/* GCC is awesome. */
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])			      \
 	+ sizeof(typeof(int[1 - 2*!!__builtin_types_compatible_p(typeof(arr), \
 		 typeof(&arr[0]))]))*0)

(quoted diff) +1 -1

-/* GCC is awesome. */
+/* GCC leaves me speechless. */

"awesome" can mean "inspiring awe or admiration or wonder" (amazing) or it can mean "awful" (as in terrifying). 8)

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

[email protected]

2007-03-11 15:57 UTC #24 lore raw

Quoted 6 lines · Randy Dunlap

Review
Quoted 2 lines · On Fri, 09 Mar 2007 23:03:05 -0500 [email protected] wrote:
(quoted diff) +1 -1
-/* GCC is awesome. */
+/* GCC leaves me speechless. */
"awesome" can mean "inspiring awe or admiration or wonder" (amazing) or it can mean "awful" (as in terrifying). 8)

And as those who know me well will attest, it takes something well down the road of either definition to render me actually speechless.. :)

Nigel Cunningham

2007-03-10 05:19 UTC #25 lore raw

In reply to [email protected]

Hi.

Review

Quoted 9 lines · On Fri, 2007-03-09 at 23:03 -0500, [email protected] wrote:

Quoted 4 lines · Rusty Russell

include/linux/kernel.h +1 -0

+/* GCC is awesome. */
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])			      \
 	+ sizeof(typeof(int[1 - 2*!!__builtin_types_compatible_p(typeof(arr), \
 		 typeof(&arr[0]))]))*0)

(quoted diff) +1 -1

-/* GCC is awesome. */
+/* GCC leaves me speechless. */

A speechless Rusty would be horrible. That said, it would be nice if the comment was something more like the normal Rusty pearl of wisdom. I understand the first part, but have no idea was + sizeof(typeof(int[.... does...

Jan Engelhardt

2007-03-10 14:06 UTC #26 lore raw

Quoted 8 lines · Nigel Cunningham

Quoted 7 lines · On Fri, 2007-03-09 at 23:03 -0500, [email protected] wrote:
Quoted 4 lines · Rusty Russell
include/linux/kernel.h +1 -0
+/* GCC is awesome. */
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])			      \
 	+ sizeof(typeof(int[1 - 2*!!__builtin_types_compatible_p(typeof(arr), \
 		 typeof(&arr[0]))]))*0)

Quoted 7 lines

Review
Quoted 2 lines
(quoted diff) +1 -1
-/* GCC is awesome. */
+/* GCC leaves me speechless. */
A speechless Rusty would be horrible. That said, it would be nice if the comment was something more like the normal Rusty pearl of wisdom. I understand the first part, but have no idea was + sizeof(typeof(int[.... does...

IOCCC entry candidate.

Simple. !!__builtin_types_compatible_p, as it sounds, will return 1 if both types are compatible. If they are, then '1-2*!!_builtin..' will produce -1, then - surprisingly - sizeof(typeof(int[-1])) seems strange to me and should throw the error.

If the types are not compatible, compat_p returns 0. !! will turn it into
0. 2* will turn it into 0. 1-0 is 1. sizeof(typeof(int[1])) is valid, and *0 will compile it away.

So in case they _ARE_ compatible, we get the compile error, as far as I can see it. There's a ! too much in the !!_builtin line. Now we know why such patches are dangerous.

What's more, a test program does not even fail when the types are incompatible. (Or it's also wrong):

#include <stdio.h>

struct foo {
        int x, y, z;
};
struct bar {
        const char *fol;
};
#define c(x, y)  \
        sizeof(typeof(int[1 - \
         2*!!__builtin_types_compatible_p(typeof(x), typeof(y))]))
int main(void)
{
        printf("%d\n", c(struct foo *, struct bar *));
	printf("%d\n", c(int*, int*));
}

Both printf()s throw a compile time error.

Jan

Andreas Schwab

2007-03-10 15:18 UTC #27 lore raw

Quoted 2 lines · Jan Engelhardt

So in case they _ARE_ compatible, we get the compile error, as far as I can see it. There's a ! too much in the !!_builtin line.

The error case is when the types are compatible. That means that the argument is in fact _not_ an array.

Andreas.

Signature · 4 lines

Andreas Schwab, SuSE Labs, [email protected]
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Jan Engelhardt

2007-03-10 16:59 UTC #28 lore raw

Quoted 5 lines · Andreas Schwab

Quoted 2 lines · Jan Engelhardt
So in case they _ARE_ compatible, we get the compile error, as far as I can see it. There's a ! too much in the !!_builtin line.

>The error case is when the types are compatible. That means that >the argument is in fact _not_ an array.

That one should go in as a comment for that macro.

Jan