Preventing ISO C errors when using macros for builtin types

Preventing ISO C errors when using macros for builtin types

[Jozef Lawrynowicz]

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

Re: Preventing ISO C errors when using macros for builtin types

[Segher Boessenkool]

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

Re: Preventing ISO C errors when using macros for builtin types

[Jozef Lawrynowicz]

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

Re: Preventing ISO C errors when using macros for builtin types

[Segher Boessenkool]

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

Re: Preventing ISO C errors when using macros for builtin types

[Jozef Lawrynowicz]

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

Re: Preventing ISO C errors when using macros for builtin types

[Richard Biener]

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

Re: Preventing ISO C errors when using macros for builtin types

[Jozef Lawrynowicz]

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

Re: Preventing ISO C errors when using macros for builtin types

[Segher Boessenkool]

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

Re: Preventing ISO C errors when using macros for builtin types

[Jozef Lawrynowicz]

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

Re: Preventing ISO C errors when using macros for builtin types

[Segher Boessenkool]

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

Re: Preventing ISO C errors when using macros for builtin types

[Jozef Lawrynowicz]

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

Re: Preventing ISO C errors when using macros for builtin types

[Segher Boessenkool]

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

Re: Preventing ISO C errors when using macros for builtin types

[Jozef Lawrynowicz]

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