* Preventing ISO C errors when using macros for builtin types @ 2019-06-05 13:26 Jozef Lawrynowicz 2019-06-05 16:49 ` Segher Boessenkool 2019-06-06 8:09 ` Richard Biener 0 siblings, 2 replies; 13+ messages in thread From: Jozef Lawrynowicz @ 2019-06-05 13:26 UTC (permalink / raw) To: gcc The MSP430 target in the large memory model uses the (non-ISO) __int20 type for SIZE_TYPE and PTRDIFF_TYPE. The preprocessor therefore expands a builtin such as __SIZE_TYPE__ to "__int20 unsigned" in user code. When compiling with the "-pedantic-errors" flag, the use of any of these builtin macros results in an error of the form: > tester.c:4:9: error: ISO C does not support '__int20' types [-Wpedantic] The GCC documentation does instruct users *not* to use these types directly (cpp.texi): > You should not use these macros directly; instead, include the > appropriate headers and use the typedefs. When using the typedefs (e.g. size_t) in a program compiled with -pedantic-errors, there is no ISO C error. However, in the testsuite there is an ever-growing list of tests which use the macros to avoid having to include any header files required for the typedefs. Since -pendantic-errors is often passed as a default flag in the testsuite, there are many false failures when testing with -mlarge, caused by this ISO C error. I would like to try to find a way to address this issue within GCC itself, so that constant updates to the testsuite are not required to filter these types of failures out. I tried one approach suggested here https://gcc.gnu.org/ml/gcc-patches/2018-11/msg02219.html which was to add "__extension__" to the definition of SIZE_TYPE/PTRDIFF_TYPE in msp430.h, however it became clear that that will not work, since the following is not valid: > typedef __extension__ __int20 ptrdiff_t; > error: expected identifier or '(' before '__extension__' __extension__ must be placed at the beginning of the declaration. I'm assuming it would not be valid to modify the behaviour of __extension__ so it can be placed within a declaration, and not just at the beginning. However, there is minimal documentation on this keyword (it does not state that it can be used in declarations, even though it can), so I wonder what the "rules" are. I would appreciate if anyone can help me decide if: - It would be OK for the use of builtin macros such as __SIZE_TYPE__ to somehow not trigger the "pedantic errors", and what a valid approach might look like * would finding a way to sandwich __extension__ into the expansion of these macros be acceptable? or, - These types of failures should be continued to be fixed in the tests themselves, for example by adding __extension__ before their usage. Thanks, Jozef ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Preventing ISO C errors when using macros for builtin types 2019-06-05 13:26 Preventing ISO C errors when using macros for builtin types Jozef Lawrynowicz @ 2019-06-05 16:49 ` Segher Boessenkool 2019-06-05 19:49 ` Jozef Lawrynowicz 2019-06-06 8:09 ` Richard Biener 1 sibling, 1 reply; 13+ messages in thread From: Segher Boessenkool @ 2019-06-05 16:49 UTC (permalink / raw) To: Jozef Lawrynowicz; +Cc: gcc On Wed, Jun 05, 2019 at 02:25:59PM +0100, Jozef Lawrynowicz wrote: > I'm assuming it would not be valid to modify the behaviour of __extension__ > so it can be placed within a declaration, and not just at the > beginning. However, there is minimal documentation on this keyword (it does not > state that it can be used in declarations, even though it can), so I wonder > what the "rules" are. The documentation says '-pedantic' and other options cause warnings for many GNU C extensions. You can prevent such warnings within one expression by writing '__extension__' before the expression. '__extension__' has no effect aside from this. It's not clear to me why you cannot simply put __extension__ earlier in your case? Segher ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Preventing ISO C errors when using macros for builtin types 2019-06-05 16:49 ` Segher Boessenkool @ 2019-06-05 19:49 ` Jozef Lawrynowicz 2019-06-05 22:12 ` Segher Boessenkool 0 siblings, 1 reply; 13+ messages in thread From: Jozef Lawrynowicz @ 2019-06-05 19:49 UTC (permalink / raw) To: Segher Boessenkool; +Cc: gcc On Wed, 5 Jun 2019 11:49:21 -0500 Segher Boessenkool <segher@kernel.crashing.org> wrote: > On Wed, Jun 05, 2019 at 02:25:59PM +0100, Jozef Lawrynowicz wrote: > > I'm assuming it would not be valid to modify the behaviour of __extension__ > > so it can be placed within a declaration, and not just at the > > beginning. However, there is minimal documentation on this keyword (it does not > > state that it can be used in declarations, even though it can), so I wonder > > what the "rules" are. > > The documentation says > > '-pedantic' and other options cause warnings for many GNU C extensions. > You can prevent such warnings within one expression by writing > '__extension__' before the expression. '__extension__' has no effect > aside from this. > > It's not clear to me why you cannot simply put __extension__ earlier in > your case? If I am modifying tests on an individual basis, then indeed I can put __extension__ earlier in the declaration and it fixes the issue. But for a fix within the compiler to prevent having to modify individual tests, I could add __extension__ to the beginning of the macro - but the macro may not end up at the beginning of a declaration in the source code. For example, say __SIZE_TYPE__ now expands to "__extension__ __int20 unsigned", then the following usage of __SIZE_TYPE__ would be ok, as __extension__ is at the beginning of the declaration: > __SIZE_TYPE__ size; But the following would emit an error, because __extension__ is not at the beginning of the declaration: > typedef __SIZE_TYPE__ size_t; I'm mainly just wondering if a change to the compiler to allow the usage of __extension__ within a declaration would be allowed. However since there are many contexts where usage of a type such as __SIZE_TYPE__ is valid, but where __extension__ preceding the type wouldn't be valid, this is probably not worth exploring.. I'm aware of some different ways that the tests can be fixed up to prevent the ISO C errors caused by -pedantic-errors. I'm currently thinking the best way to fix these issues in the testsuite might be to implement a directive such as "dg-prune-options". This directive can then prune "-pedantic-errors" from the options to be passed to GCC when some specified condition (e.g. -mlarge is in the opt list) is matched. This would at least avoid having to re-jig tests to get the __extension__ keyword into the right places, as the dg-prune-options can just be added to the problematic tests. Thanks, Jozef ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Preventing ISO C errors when using macros for builtin types 2019-06-05 19:49 ` Jozef Lawrynowicz @ 2019-06-05 22:12 ` Segher Boessenkool 2019-06-10 15:57 ` Jozef Lawrynowicz 0 siblings, 1 reply; 13+ messages in thread From: Segher Boessenkool @ 2019-06-05 22:12 UTC (permalink / raw) To: Jozef Lawrynowicz; +Cc: gcc On Wed, Jun 05, 2019 at 08:49:39PM +0100, Jozef Lawrynowicz wrote: > On Wed, 5 Jun 2019 11:49:21 -0500 > Segher Boessenkool <segher@kernel.crashing.org> wrote: > > The documentation says > > > > '-pedantic' and other options cause warnings for many GNU C extensions. > > You can prevent such warnings within one expression by writing > > '__extension__' before the expression. '__extension__' has no effect > > aside from this. > > > > It's not clear to me why you cannot simply put __extension__ earlier in > > your case? > > If I am modifying tests on an individual basis, then indeed I can put > __extension__ earlier in the declaration and it fixes the issue. Or you could fix it like this in your header file? > But for a fix within the compiler to prevent having to modify individual tests, > I could add __extension__ to the beginning of the macro - but the macro may > not end up at the beginning of a declaration in the source code. > > For example, say __SIZE_TYPE__ now expands to "__extension__ __int20 unsigned", > then the following usage of __SIZE_TYPE__ would be ok, as __extension__ is at > the beginning of the declaration: > > > __SIZE_TYPE__ size; Don't use macros for types? You could use something like __extension__ typedef unsigned __int20 __SIZE_TYPE__; (stddef.h already uses typedefs like that, btw, for __SIZE_TYPE__ in fact). > I'm mainly just wondering if a change to the compiler to allow the usage of > __extension__ within a declaration would be allowed. Well, how would that work? We'd need to modify the grammar to allow __extension__ pretty much anywhere, and then change all compiler code to not pedwarn until it has parsed a full expression (or statement, or file, or however this will work). Or make it not warn for things after the __extension__ only, or make it only *guaranteed* not to warn for things *after* the __extension__. None of those options are very appetising, IMO. Segher ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Preventing ISO C errors when using macros for builtin types 2019-06-05 22:12 ` Segher Boessenkool @ 2019-06-10 15:57 ` Jozef Lawrynowicz 0 siblings, 0 replies; 13+ messages in thread From: Jozef Lawrynowicz @ 2019-06-10 15:57 UTC (permalink / raw) To: Segher Boessenkool; +Cc: gcc On Wed, 5 Jun 2019 17:12:25 -0500 Segher Boessenkool <segher@kernel.crashing.org> wrote: > On Wed, Jun 05, 2019 at 08:49:39PM +0100, Jozef Lawrynowicz wrote: > > On Wed, 5 Jun 2019 11:49:21 -0500 > > Segher Boessenkool <segher@kernel.crashing.org> wrote: > > > The documentation says > > > > > > '-pedantic' and other options cause warnings for many GNU C extensions. > > > You can prevent such warnings within one expression by writing > > > '__extension__' before the expression. '__extension__' has no effect > > > aside from this. > > > > > > It's not clear to me why you cannot simply put __extension__ earlier in > > > your case? > > > > If I am modifying tests on an individual basis, then indeed I can put > > __extension__ earlier in the declaration and it fixes the issue. > > Or you could fix it like this in your header file? If you're referring to the main target board header file (i.e. gcc/config/msp430/msp430.h), I cannot add __extension__ to the definition of SIZE_TYPE. It just breaks the build in too many places where a specific format for SIZE_TYPE is expected. Should I manage to coerce the build into succeeding, there would be further issues from the grammar rules regarding where __extension__ can be placed in expressions/declarations anyway. > > But for a fix within the compiler to prevent having to modify individual tests, > > I could add __extension__ to the beginning of the macro - but the macro may > > not end up at the beginning of a declaration in the source code. > > > > For example, say __SIZE_TYPE__ now expands to "__extension__ __int20 unsigned", > > then the following usage of __SIZE_TYPE__ would be ok, as __extension__ is at > > the beginning of the declaration: > > > > > __SIZE_TYPE__ size; > > Don't use macros for types? You could use something like > > __extension__ typedef unsigned __int20 __SIZE_TYPE__; I am only really concerned about the usage of __SIZE_TYPE__ in the GCC testsuite (since real users shouldn't be using these macros, and the user-facing typedefs do not cause ISO C errors), and it is in those tests that there is uninhibited use of __SIZE_TYPE__ directly as a type. Combined with -pedantic-errors being a default flag we see an ever expanding list of failures. So to make tracking testsuite failures easier, I was looking for a way to prevent these false errors by modifying the compiler itself. Failing that, a simple and quick modification I can make to the tests would be satisfactory - such as the aforementioned potential new DejaGNU directive to prune the -pedantic-errors option before running the test. The above typedef you suggested has in fact been similar to how I've been going about fixing these failures so far, however it occurred to me that there should be a better and more efficient way of avoiding these failures. > > I'm mainly just wondering if a change to the compiler to allow the usage of > > __extension__ within a declaration would be allowed. > > Well, how would that work? We'd need to modify the grammar to allow > __extension__ pretty much anywhere, and then change all compiler code > to not pedwarn until it has parsed a full expression (or statement, or > file, or however this will work). Or make it not warn for things after > the __extension__ only, or make it only *guaranteed* not to warn for > things *after* the __extension__. > > None of those options are very appetising, IMO. Yes I do agree with you, the mechanism for fixing this in the compiler would require significant work for this relatively niche use case. Thanks, Jozef ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Preventing ISO C errors when using macros for builtin types 2019-06-05 13:26 Preventing ISO C errors when using macros for builtin types Jozef Lawrynowicz 2019-06-05 16:49 ` Segher Boessenkool @ 2019-06-06 8:09 ` Richard Biener 2019-06-10 16:20 ` Jozef Lawrynowicz 1 sibling, 1 reply; 13+ messages in thread From: Richard Biener @ 2019-06-06 8:09 UTC (permalink / raw) To: Jozef Lawrynowicz, David Malcolm; +Cc: GCC Development On Wed, Jun 5, 2019 at 3:26 PM Jozef Lawrynowicz <jozefl.gcc@gmail.com> wrote: > > The MSP430 target in the large memory model uses the (non-ISO) __int20 type for > SIZE_TYPE and PTRDIFF_TYPE. > The preprocessor therefore expands a builtin such as __SIZE_TYPE__ to > "__int20 unsigned" in user code. > When compiling with the "-pedantic-errors" flag, the use of any of these > builtin macros results in an error of the form: > > > tester.c:4:9: error: ISO C does not support '__int20' types [-Wpedantic] > > The GCC documentation does instruct users *not* to use these types directly > (cpp.texi): > > You should not use these macros directly; instead, include the > > appropriate headers and use the typedefs. > When using the typedefs (e.g. size_t) in a program compiled with > -pedantic-errors, there is no ISO C error. > > However, in the testsuite there is an ever-growing list of tests which use > the macros to avoid having to include any header files required for the > typedefs. > Since -pendantic-errors is often passed as a default flag in the testsuite, > there are many false failures when testing with -mlarge, caused by this ISO C > error. > > I would like to try to find a way to address this issue within GCC itself, so > that constant updates to the testsuite are not required to filter these types > of failures out. > > I tried one approach suggested here > https://gcc.gnu.org/ml/gcc-patches/2018-11/msg02219.html > which was to add "__extension__" to the definition of SIZE_TYPE/PTRDIFF_TYPE in > msp430.h, however it became clear that that will not work, since the following > is not valid: > > typedef __extension__ __int20 ptrdiff_t; > > > error: expected identifier or '(' before '__extension__' > > __extension__ must be placed at the beginning of the declaration. > > I'm assuming it would not be valid to modify the behaviour of __extension__ > so it can be placed within a declaration, and not just at the > beginning. However, there is minimal documentation on this keyword (it does not > state that it can be used in declarations, even though it can), so I wonder > what the "rules" are. > > I would appreciate if anyone can help me decide if: > - It would be OK for the use of builtin macros such as __SIZE_TYPE__ to somehow > not trigger the "pedantic errors", and what a valid approach might look like I think that would be OK - note you could also modify your target board. Maybe we can trick libcpp to set in_system_header for those internal predefined macros so we do not warn on their expansions. We can also lookup macro expansion context and eventually see it is an internal macro. people more familiar with libcpp could say which approach is more reasonable. Richard. > * would finding a way to sandwich __extension__ into the expansion of these > macros be acceptable? > or, > - These types of failures should be continued to be fixed in the tests > themselves, for example by adding __extension__ before their usage. > > Thanks, > Jozef ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Preventing ISO C errors when using macros for builtin types 2019-06-06 8:09 ` Richard Biener @ 2019-06-10 16:20 ` Jozef Lawrynowicz 2019-06-10 18:32 ` Segher Boessenkool 0 siblings, 1 reply; 13+ messages in thread From: Jozef Lawrynowicz @ 2019-06-10 16:20 UTC (permalink / raw) To: Richard Biener; +Cc: David Malcolm, GCC Development, Segher Boessenkool On Thu, 6 Jun 2019 10:09:32 +0200 Richard Biener <richard.guenther@gmail.com> wrote: > On Wed, Jun 5, 2019 at 3:26 PM Jozef Lawrynowicz <jozefl.gcc@gmail.com> wrote: > > > > I would appreciate if anyone can help me decide if: > > - It would be OK for the use of builtin macros such as __SIZE_TYPE__ to somehow > > not trigger the "pedantic errors", and what a valid approach might look like > > I think that would be OK - note you could also modify your target board. I'm now realising that the most straightforward way to fix this issue will be to just modify the configuration of the DejaGNU target board, so that DEFAULT_CFLAGS is set there and declarations of DEFAULT_CFLAGS in the testsuite that would set -pedantic-errors are never used. Thanks, Jozef ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Preventing ISO C errors when using macros for builtin types 2019-06-10 16:20 ` Jozef Lawrynowicz @ 2019-06-10 18:32 ` Segher Boessenkool 2019-06-10 19:58 ` Jozef Lawrynowicz 0 siblings, 1 reply; 13+ messages in thread From: Segher Boessenkool @ 2019-06-10 18:32 UTC (permalink / raw) To: Jozef Lawrynowicz; +Cc: Richard Biener, David Malcolm, GCC Development On Mon, Jun 10, 2019 at 05:20:31PM +0100, Jozef Lawrynowicz wrote: > On Thu, 6 Jun 2019 10:09:32 +0200 > Richard Biener <richard.guenther@gmail.com> wrote: > > > On Wed, Jun 5, 2019 at 3:26 PM Jozef Lawrynowicz <jozefl.gcc@gmail.com> wrote: > > > > > > I would appreciate if anyone can help me decide if: > > > - It would be OK for the use of builtin macros such as __SIZE_TYPE__ to somehow > > > not trigger the "pedantic errors", and what a valid approach might look like > > > > I think that would be OK - note you could also modify your target board. > > I'm now realising that the most straightforward way to fix this issue will be > to just modify the configuration of the DejaGNU target board, so that > DEFAULT_CFLAGS is set there and declarations of DEFAULT_CFLAGS in the testsuite > that would set -pedantic-errors are never used. That is not a fix, that is sweeping the problem under the rug. As a somewhat dirty hack I added #if __MSP430X_LARGE__ #undef __SIZE_TYPE__ __extension__ typedef unsigned __int20 __SIZE_TYPE__; #endif to the start of the installed stddef.h, and that fixes the problem fine, for correct programs that do not forget to include <stddef.h> (directly or indirectly), anyway. Segher ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Preventing ISO C errors when using macros for builtin types 2019-06-10 18:32 ` Segher Boessenkool @ 2019-06-10 19:58 ` Jozef Lawrynowicz 2019-06-10 22:09 ` Segher Boessenkool 0 siblings, 1 reply; 13+ messages in thread From: Jozef Lawrynowicz @ 2019-06-10 19:58 UTC (permalink / raw) To: Segher Boessenkool; +Cc: Richard Biener, David Malcolm, GCC Development On Mon, 10 Jun 2019 13:32:42 -0500 Segher Boessenkool <segher@kernel.crashing.org> wrote: > On Mon, Jun 10, 2019 at 05:20:31PM +0100, Jozef Lawrynowicz wrote: > > On Thu, 6 Jun 2019 10:09:32 +0200 > > Richard Biener <richard.guenther@gmail.com> wrote: > > > > > On Wed, Jun 5, 2019 at 3:26 PM Jozef Lawrynowicz <jozefl.gcc@gmail.com> wrote: > > > > > > > > I would appreciate if anyone can help me decide if: > > > > - It would be OK for the use of builtin macros such as __SIZE_TYPE__ to somehow > > > > not trigger the "pedantic errors", and what a valid approach might look like > > > > > > I think that would be OK - note you could also modify your target board. > > > > I'm now realising that the most straightforward way to fix this issue will be > > to just modify the configuration of the DejaGNU target board, so that > > DEFAULT_CFLAGS is set there and declarations of DEFAULT_CFLAGS in the testsuite > > that would set -pedantic-errors are never used. > > That is not a fix, that is sweeping the problem under the rug. > > As a somewhat dirty hack I added > > #if __MSP430X_LARGE__ > #undef __SIZE_TYPE__ > __extension__ typedef unsigned __int20 __SIZE_TYPE__; > #endif > > to the start of the installed stddef.h, and that fixes the problem fine, > for correct programs that do not forget to include <stddef.h> (directly > or indirectly), anyway. > > > Segher But we have some 850 generic tests in gcc/testsuite that use __SIZE_TYPE__ without including stddef.h. They just rely on the preprocessor to expand this using the builtin macro definition. I assumed it was standard (and preferred, for the sake of compilation time at least) for them to omit including stddef.h, which is why I was considering pursuing some way of modifying how the builtin macro is expanded. I could add something like your above code snippet to the tests that trigger the ISO C error, but this is what I was originally trying to avoid. At least, it seems like a good number of tests have the following: typedef __SIZE_TYPE__ size_t; For these the fixup is simple as we can just add __extension__ before the typedef. For others (e.g. gcc.dg/c99-const-expr-10.c), either we - add something like your snippet above and redefine __SIZE_TYPE__, or - replace uses of __SIZE_TYPE__ with size_t and add the typedef i.e. __extension__ typedef __SIZE_TYPE__ size_t; Do you have any opinion on which of these would be preferred? For modifying the individual tests, I would assume it would be to add the required typedefs at the top of the file, as this is what some tests already do. It also avoids having the target specific caveats in the test file and may help other targets with non-ISO types. Thanks, Jozef ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Preventing ISO C errors when using macros for builtin types 2019-06-10 19:58 ` Jozef Lawrynowicz @ 2019-06-10 22:09 ` Segher Boessenkool 2019-06-11 20:44 ` Jozef Lawrynowicz 0 siblings, 1 reply; 13+ messages in thread From: Segher Boessenkool @ 2019-06-10 22:09 UTC (permalink / raw) To: Jozef Lawrynowicz; +Cc: Richard Biener, David Malcolm, GCC Development On Mon, Jun 10, 2019 at 08:58:00PM +0100, Jozef Lawrynowicz wrote: > On Mon, 10 Jun 2019 13:32:42 -0500 > Segher Boessenkool <segher@kernel.crashing.org> wrote: > > That is not a fix, that is sweeping the problem under the rug. > > > > As a somewhat dirty hack I added > > > > #if __MSP430X_LARGE__ > > #undef __SIZE_TYPE__ > > __extension__ typedef unsigned __int20 __SIZE_TYPE__; > > #endif > > > > to the start of the installed stddef.h, and that fixes the problem fine, > > for correct programs that do not forget to include <stddef.h> (directly > > or indirectly), anyway. > But we have some 850 generic tests in gcc/testsuite that use __SIZE_TYPE__ > without including stddef.h. They just rely on the preprocessor to expand this > using the builtin macro definition. I did say it is a dirty hack, right? You could call lang_hooks.types.register_builtin_type defining the type __SIZE_TYPE__ as the int20 partial int type, and then define SIZE_TYPE as just __SIZE_TYPE__. That is the same effectively, just not using the header file. So something like tree type_node = builtin_type_for_size (20, 1); lang_hooks.types.register_builtin_type (type_node, "__SIZE_TYPE__"); or maybe tree type_node = builtin_type_for_size (POINTER_SIZE, 1); lang_hooks.types.register_builtin_type (type_node, "__SIZE_TYPE__"); in msp430.c, and #define SIZE_TYPE "__SIZE_TYPE__" in msp430.h? Segher ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Preventing ISO C errors when using macros for builtin types 2019-06-10 22:09 ` Segher Boessenkool @ 2019-06-11 20:44 ` Jozef Lawrynowicz 2019-06-11 23:01 ` Segher Boessenkool 0 siblings, 1 reply; 13+ messages in thread From: Jozef Lawrynowicz @ 2019-06-11 20:44 UTC (permalink / raw) To: Segher Boessenkool; +Cc: Richard Biener, David Malcolm, GCC Development [-- Attachment #1: Type: text/plain, Size: 3486 bytes --] On Mon, 10 Jun 2019 17:09:10 -0500 Segher Boessenkool <segher@kernel.crashing.org> wrote: > On Mon, Jun 10, 2019 at 08:58:00PM +0100, Jozef Lawrynowicz wrote: > > On Mon, 10 Jun 2019 13:32:42 -0500 > > Segher Boessenkool <segher@kernel.crashing.org> wrote: > > > That is not a fix, that is sweeping the problem under the rug. > > > > > > As a somewhat dirty hack I added > > > > > > #if __MSP430X_LARGE__ > > > #undef __SIZE_TYPE__ > > > __extension__ typedef unsigned __int20 __SIZE_TYPE__; > > > #endif > > > > > > to the start of the installed stddef.h, and that fixes the problem fine, > > > for correct programs that do not forget to include <stddef.h> (directly > > > or indirectly), anyway. > > > But we have some 850 generic tests in gcc/testsuite that use __SIZE_TYPE__ > > without including stddef.h. They just rely on the preprocessor to expand this > > using the builtin macro definition. > > I did say it is a dirty hack, right? > > You could call lang_hooks.types.register_builtin_type defining the type > __SIZE_TYPE__ as the int20 partial int type, and then define SIZE_TYPE as > just __SIZE_TYPE__. That is the same effectively, just not using the > header file. > > So something like > > tree type_node = builtin_type_for_size (20, 1); > lang_hooks.types.register_builtin_type (type_node, "__SIZE_TYPE__"); > > or maybe > > tree type_node = builtin_type_for_size (POINTER_SIZE, 1); > lang_hooks.types.register_builtin_type (type_node, "__SIZE_TYPE__"); > > in msp430.c, and > > #define SIZE_TYPE "__SIZE_TYPE__" > > in msp430.h? > > > Segher Thanks for the pointers, they've put me on the right track I think. It doesn't look like we can create the new type in the msp430 backend - the SIZE_TYPE macro is examined quite early and only a couple of basic backend initialization functions are called before SIZE_TYPE is needed in c_common_nodes_and_builtins(). TARGET_INIT_BUILTINS seemed most appropriate, but by then it's too late to create the type and use it in msp430.h. Instead I fixed it by creating a new type "__int20__" in the middle-end, which can then be used for SIZE_TYPE in msp430.h. __int20__ is not really a proper type - it shares it's "RID" values with __int20, but it means we can gate the ISO C pedwarn so it only is emitted for __int20 and not __int20__. It's ok for __int20 and __int20__ to have identical behavior, aside from the pedwarn, which is why sharing the RIDs should be ok. I think this solution fits ok with the existing behavior related to "pedantic" warnings, since alternate keywords beginning and ending with "__" are considered GNU Extensions and don't pedwarn. I guess "__int20" isn't technically a "keyword" in the same sense as "asm" for example. But its a "reserved word" and this fix fits this pattern of surrounding with double-underscores. Any thoughts on this approach in my attached (rough) patch? So far I regtested it fine with the GCC C testsuite for msp430-elf. Also need to do the same for PTRDIFF_TYPE and make that use __int20__. I initially implemented this by giving the __intN__ types their own new set of RIDs so they are completely distinct from __intN, but it requires quite a lot of duplicated code whenever RID_INT_N_* or RID_{FIRST,LAST}_INT_N are used, just to handle the new RIDs. This patch is at least quite concise and gets the job done. Of course, if the __intN__ types really should have their own unique RIDs then I can do it that way instead. Thanks, Jozef [-- Attachment #2: implement-__int20__.patch --] [-- Type: text/x-patch, Size: 5166 bytes --] diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 4057be3aaed..57e84b84f07 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -4024,8 +4024,14 @@ c_common_nodes_and_builtins (void) sprintf (name, "__int%d", int_n_data[i].bitsize); record_builtin_type ((enum rid)(RID_FIRST_INT_N + i), name, int_n_trees[i].signed_type); + sprintf (name, "__int%d__", int_n_data[i].bitsize); + record_builtin_type ((enum rid)(RID_FIRST_INT_N + i), name, + int_n_trees[i].signed_type); + sprintf (name, "__int%d unsigned", int_n_data[i].bitsize); record_builtin_type (RID_MAX, name, int_n_trees[i].unsigned_type); + sprintf (name, "__int%d__ unsigned", int_n_data[i].bitsize); + record_builtin_type (RID_MAX, name, int_n_trees[i].unsigned_type); } if (c_dialect_cxx ()) diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c index 87ce853d4b7..cb2f49fa5a2 100644 --- a/gcc/c/c-decl.c +++ b/gcc/c/c-decl.c @@ -10637,7 +10637,11 @@ declspecs_add_type (location_t loc, struct c_declspecs *specs, case RID_INT_N_2: case RID_INT_N_3: specs->int_n_idx = i - RID_INT_N_0; - if (!in_system_header_at (input_location)) + if (!in_system_header_at (input_location) + /* If the INT_N type ends in "__", and so is of the format + "__intN__", don't pedwarn. */ + && (strncmp (IDENTIFIER_POINTER (type) + + (IDENTIFIER_LENGTH (type) - 2), "__", 2) != 0)) pedwarn (loc, OPT_Wpedantic, "ISO C does not support %<__int%d%> types", int_n_data[specs->int_n_idx].bitsize); diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c index df1a3042276..98508721ed9 100644 --- a/gcc/c/c-parser.c +++ b/gcc/c/c-parser.c @@ -157,6 +157,11 @@ c_parse_init (void) id = get_identifier (name); C_SET_RID_CODE (id, RID_FIRST_INT_N + i); C_IS_RESERVED_WORD (id) = 1; + + sprintf (name, "__int%d__", int_n_data[i].bitsize); + id = get_identifier (name); + C_SET_RID_CODE (id, RID_FIRST_INT_N + i); + C_IS_RESERVED_WORD (id) = 1; } } \f diff --git a/gcc/config/msp430/msp430.h b/gcc/config/msp430/msp430.h index c0aa8eacd96..689ed26f5e2 100644 --- a/gcc/config/msp430/msp430.h +++ b/gcc/config/msp430/msp430.h @@ -185,7 +185,7 @@ extern const char * msp430_select_hwmult_lib (int, const char **); /* Layout of Source Language Data Types */ #undef SIZE_TYPE -#define SIZE_TYPE (TARGET_LARGE ? "__int20 unsigned" : "unsigned int") +#define SIZE_TYPE (TARGET_LARGE ? "__int20__ unsigned" : "unsigned int") #undef PTRDIFF_TYPE #define PTRDIFF_TYPE (TARGET_LARGE ? "__int20" : "int") #undef WCHAR_TYPE diff --git a/gcc/cp/lex.c b/gcc/cp/lex.c index 20965e49fe4..526c95e3539 100644 --- a/gcc/cp/lex.c +++ b/gcc/cp/lex.c @@ -262,6 +262,11 @@ init_reswords (void) id = get_identifier (name); C_SET_RID_CODE (id, RID_FIRST_INT_N + i); set_identifier_kind (id, cik_keyword); + + sprintf (name, "__int%d__", int_n_data[i].bitsize); + id = get_identifier (name); + C_SET_RID_CODE (id, RID_FIRST_INT_N + i); + set_identifier_kind (id, cik_keyword); } } diff --git a/gcc/lto/lto-lang.c b/gcc/lto/lto-lang.c index e155ea33d32..2769719995b 100644 --- a/gcc/lto/lto-lang.c +++ b/gcc/lto/lto-lang.c @@ -1260,9 +1260,9 @@ lto_build_c_type_nodes (void) if (int_n_enabled_p[i]) { char name[50]; - sprintf (name, "__int%d unsigned", int_n_data[i].bitsize); + sprintf (name, "int%d", int_n_data[i].bitsize); - if (strcmp (name, SIZE_TYPE) == 0) + if (strstr (SIZE_TYPE, name) != NULL) { intmax_type_node = int_n_trees[i].signed_type; uintmax_type_node = int_n_trees[i].unsigned_type; diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c index 5d6f2e0166c..b9d60823a68 100644 --- a/gcc/stor-layout.c +++ b/gcc/stor-layout.c @@ -2717,9 +2717,9 @@ initialize_sizetypes (void) if (int_n_enabled_p[i]) { char name[50]; - sprintf (name, "__int%d unsigned", int_n_data[i].bitsize); + sprintf (name, "int%d", int_n_data[i].bitsize); - if (strcmp (name, SIZETYPE) == 0) + if (strstr (SIZETYPE, name) != NULL) { precision = int_n_data[i].bitsize; } diff --git a/gcc/tree.c b/gcc/tree.c index e879f15a841..9a7cb476ae4 100644 --- a/gcc/tree.c +++ b/gcc/tree.c @@ -10383,9 +10383,9 @@ build_common_tree_nodes (bool signed_char) if (int_n_enabled_p[i]) { char name[50]; - sprintf (name, "__int%d unsigned", int_n_data[i].bitsize); + sprintf (name, "int%d", int_n_data[i].bitsize); - if (strcmp (name, SIZE_TYPE) == 0) + if (strstr (SIZE_TYPE, name) != NULL) { size_type_node = int_n_trees[i].unsigned_type; } @@ -10410,8 +10410,9 @@ build_common_tree_nodes (bool signed_char) if (int_n_enabled_p[i]) { char name[50]; - sprintf (name, "__int%d", int_n_data[i].bitsize); - if (strcmp (name, PTRDIFF_TYPE) == 0) + sprintf (name, "int%d", int_n_data[i].bitsize); + + if (strstr (PTRDIFF_TYPE, name) != NULL) ptrdiff_type_node = int_n_trees[i].signed_type; } if (ptrdiff_type_node == NULL_TREE) -- 2.17.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Preventing ISO C errors when using macros for builtin types 2019-06-11 20:44 ` Jozef Lawrynowicz @ 2019-06-11 23:01 ` Segher Boessenkool 2019-06-12 16:40 ` Jozef Lawrynowicz 0 siblings, 1 reply; 13+ messages in thread From: Segher Boessenkool @ 2019-06-11 23:01 UTC (permalink / raw) To: Jozef Lawrynowicz; +Cc: Richard Biener, David Malcolm, GCC Development Hi Jozef, On Tue, Jun 11, 2019 at 09:44:30PM +0100, Jozef Lawrynowicz wrote: > Thanks for the pointers, they've put me on the right track I think. > > It doesn't look like we can create the new type in the msp430 backend - the > SIZE_TYPE macro is examined quite early and only a couple of basic backend > initialization functions are called before SIZE_TYPE is needed in > c_common_nodes_and_builtins(). TARGET_INIT_BUILTINS seemed most appropriate, > but by then it's too late to create the type and use it in msp430.h. > > Instead I fixed it by creating a new type "__int20__" in the middle-end, > which can then be used for SIZE_TYPE in msp430.h. > __int20__ is not really a proper type - it shares it's "RID" values with > __int20, but it means we can gate the ISO C pedwarn so it only is emitted for > __int20 and not __int20__. Ooh I like this :-) Fits in well everywhere, and it's nicely generic, too. > It's ok for __int20 and __int20__ to have identical > behavior, aside from the pedwarn, which is why sharing the RIDs should be ok. Many other keywords do the same, see asm/__asm/__asm__ for example. > I think this solution fits ok with the existing behavior related to "pedantic" > warnings, since alternate keywords beginning and ending > with "__" are considered GNU Extensions and don't pedwarn. I guess "__int20" > isn't technically a "keyword" in the same sense as "asm" for example. But > its a "reserved word" and this fix fits this pattern of surrounding with > double-underscores. > Any thoughts on this approach in my attached (rough) patch? So far I regtested > it fine with the GCC C testsuite for msp430-elf. > Also need to do the same for PTRDIFF_TYPE and make that use __int20__. > --- a/gcc/lto/lto-lang.c > +++ b/gcc/lto/lto-lang.c > @@ -1260,9 +1260,9 @@ lto_build_c_type_nodes (void) > if (int_n_enabled_p[i]) > { > char name[50]; > - sprintf (name, "__int%d unsigned", int_n_data[i].bitsize); > + sprintf (name, "int%d", int_n_data[i].bitsize); > > - if (strcmp (name, SIZE_TYPE) == 0) > + if (strstr (SIZE_TYPE, name) != NULL) > { > intmax_type_node = int_n_trees[i].signed_type; > uintmax_type_node = int_n_trees[i].unsigned_type; I don't think that is correct, strstr allows too much? If you want to allow some variants, you should test for all those variants exactly? It looks great otherwise :-) Segher ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Preventing ISO C errors when using macros for builtin types 2019-06-11 23:01 ` Segher Boessenkool @ 2019-06-12 16:40 ` Jozef Lawrynowicz 0 siblings, 0 replies; 13+ messages in thread From: Jozef Lawrynowicz @ 2019-06-12 16:40 UTC (permalink / raw) To: Segher Boessenkool; +Cc: Richard Biener, David Malcolm, GCC Development On Tue, 11 Jun 2019 18:01:55 -0500 Segher Boessenkool <segher@kernel.crashing.org> wrote: > On Tue, Jun 11, 2019 at 09:44:30PM +0100, Jozef Lawrynowicz wrote: > > --- a/gcc/lto/lto-lang.c > > +++ b/gcc/lto/lto-lang.c > > @@ -1260,9 +1260,9 @@ lto_build_c_type_nodes (void) > > if (int_n_enabled_p[i]) > > { > > char name[50]; > > - sprintf (name, "__int%d unsigned", int_n_data[i].bitsize); > > + sprintf (name, "int%d", int_n_data[i].bitsize); > > > > - if (strcmp (name, SIZE_TYPE) == 0) > > + if (strstr (SIZE_TYPE, name) != NULL) > > { > > intmax_type_node = int_n_trees[i].signed_type; > > uintmax_type_node = int_n_trees[i].unsigned_type; > > I don't think that is correct, strstr allows too much? If you want to > allow some variants, you should test for all those variants exactly? Yeah I'll fix this up in my full patch submission. > It looks great otherwise :-) > > > Segher Great, thanks for all your help! Jozef ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-06-12 16:40 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-05 13:26 Preventing ISO C errors when using macros for builtin types Jozef Lawrynowicz 2019-06-05 16:49 ` Segher Boessenkool 2019-06-05 19:49 ` Jozef Lawrynowicz 2019-06-05 22:12 ` Segher Boessenkool 2019-06-10 15:57 ` Jozef Lawrynowicz 2019-06-06 8:09 ` Richard Biener 2019-06-10 16:20 ` Jozef Lawrynowicz 2019-06-10 18:32 ` Segher Boessenkool 2019-06-10 19:58 ` Jozef Lawrynowicz 2019-06-10 22:09 ` Segher Boessenkool 2019-06-11 20:44 ` Jozef Lawrynowicz 2019-06-11 23:01 ` Segher Boessenkool 2019-06-12 16:40 ` Jozef Lawrynowicz
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).