Bug in __extension__ handling for record fields

This log shows the problem:

$ cat q.c

struct {
  __extension__ unsigned long long b;
} a;
$ ~/llvm/Debug/bin/clang-cc -pedantic -std=c89 q.c
q.c:3:31: warning: 'long long' is an extension when C99 mode is not enabled
  __extension__ unsigned long long b;
                              ^
1 diagnostic generated.

I've tracked down the problem to the fact that in
Parser::ParseStructUnionBody it's called ParseStructDeclaration that
during its execution disables the warning, but after its return the
warning is generated by Actions.ActOnField call that operates with
warnings enabled.

A way to fix could be to add a flag to FieldDeclarator to remember that
the declaration was marked by __extension__.

The flag would be set by ParseStructDeclaration and then read in the
following loop to disable again the warnings for the execution of
Actions.ActOnField.

If you agree with this approach I'm willing to submit a patch.

This sounds like a reasonable approach to me!

-Chris

Chris Lattner ha scritto:

This log shows the problem:

$ cat q.c

struct {
__extension__ unsigned long long b;
} a;
$ ~/llvm/Debug/bin/clang-cc -pedantic -std=c89 q.c
q.c:3:31: warning: 'long long' is an extension when C99 mode is not
enabled
__extension__ unsigned long long b;
                             ^
1 diagnostic generated.

This sounds like a reasonable approach to me!

I've attached the patch.

I've preferred to put the flag in Declarator to have it available for a
wider range of Declaration.

Once this patch is commited (and if you agree) I'd like to submit a
further patch to map the saved extension flag in an appropriate
attribute to save this info in the AST (there is a FIXME somewhere in
clang stating this needs).

extension.patch (3.41 KB)

Abramo Bagnara ha scritto:

Chris Lattner ha scritto:

This log shows the problem:

$ cat q.c

struct {
__extension__ unsigned long long b;
} a;
$ ~/llvm/Debug/bin/clang-cc -pedantic -std=c89 q.c
q.c:3:31: warning: 'long long' is an extension when C99 mode is not
enabled
__extension__ unsigned long long b;
                             ^
1 diagnostic generated.

This sounds like a reasonable approach to me!

I've attached the patch.

I've preferred to put the flag in Declarator to have it available for a
wider range of Declaration.

Once this patch is commited (and if you agree) I'd like to submit a
further patch to map the saved extension flag in an appropriate
attribute to save this info in the AST (there is a FIXME somewhere in
clang stating this needs).

Everything is ok with the sent patch?

Hello.

I would like to know what is the status of the patch that was submitted by Abramo on August 15th:

   http://lists.cs.uiuc.edu/pipermail/cfe-dev/2009-August/006118.html

Is this still under review?

I just tried it on r79924 and it seems to be working OK, except for a single test that fails but, afaict, it should not (since the test uses the keyword __extension__, the expected warning should be silenced).

So, find attached the improved patch, which is the same as that of August 15th by Abramo plus the correction for the test mentioned above.

Please let us know if there is anything requiring improvement or correction.

Cheers,
Enea Zaffanella.

Extension-2.patch (3.79 KB)

Hello.

I would like to know what is the status of the patch that was submitted by Abramo on August 15th:

http://lists.cs.uiuc.edu/pipermail/cfe-dev/2009-August/006118.html

Is this still under review?

It slipped by while I was on vacation. Sorry Abramo!

I just tried it on r79924 and it seems to be working OK, except for a single test that fails but, afaict, it should not (since the test uses the keyword __extension__, the expected warning should be silenced).

So, find attached the improved patch, which is the same as that of August 15th by Abramo plus the correction for the test mentioned above.

Looks good; I've committed it here:

  http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20090824/020655.html

Thanks to Abramo and Enea!

  - Doug

Douglas Gregor ha scritto:

Hello.

I would like to know what is the status of the patch that was
submitted by Abramo on August 15th:

http://lists.cs.uiuc.edu/pipermail/cfe-dev/2009-August/006118.html

Is this still under review?

It slipped by while I was on vacation. Sorry Abramo!

I've also another pending request for a possible patch submission
approval from many days ago about EvaluateAsLValue.

Could you take a look?

This is the link to the archive:
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2009-August/006159.html