Re: [PATCH] c: Handle scoped attributes in __has*attribute and scoped attribute parsing changes in -std=c11 etc. modes [PR114007]
On Thu, 22 Feb 2024, Jakub Jelinek wrote: > But sure, if you prefer the COLON_SCOPE version of the patch, I can commit > that. There is no PREV_WHITE in the preprocessor, there is Yes, I prefer the COLON_SCOPE version. -- Joseph S. Myers josmy...@redhat.com
Re: [PATCH] c: Handle scoped attributes in __has*attribute and scoped attribute parsing changes in -std=c11 etc. modes [PR114007]
On Thu, Feb 22, 2024 at 05:49:12PM +, Joseph Myers wrote:
> This patch (the one using COLON_SCOPE, *not* the one using PREV_WHITE) is
> OK.
>
> PREV_WHITE is about whether there is whitespace between the tokens in the
> macro expansion, for the purposes of stringization - I don't think it's
> appropriate to use here. For example, given
>
> #define COLON() :
>
> then
>
> [[gnu COLON()COLON() unused]] int x;
>
> should preferably not be valid; stringizing the results of expanding
> COLON()COLON() will produce "::" (PREV_WHITE not set), but that wouldn't
> be a valid attribute in C23 and I don't think anyone could reasonably
> expect it to be valid with previous standard versions.
It is not valid in either version of the patch, with any of -std=c11,
-std=gnu11 or -std=c23 with both compilers I get the same
/tmp/test.c:2:1: warning: ‘gnu’ attribute ignored [-Wattributes]
2 | [[gnu COLON()COLON() unused]] int x;
| ^
/tmp/test.c:2:6: error: expected ‘]’ before ‘:’ token
2 | [[gnu COLON()COLON() unused]] int x;
| ^
| ]
message all the time.
But sure, if you prefer the COLON_SCOPE version of the patch, I can commit
that. There is no PREV_WHITE in the preprocessor, there is
CPP_PADDING token in between the CPP_COLONs though and that translates
into PREV_WHITE on the second CPP_COLON in the FE token.
When trying to verify it, I've noticed one typo in the PREV_WHITE patch:
@@ -73,7 +73,7 @@ gcc/testsuite/
switch (type)
{
case CPP_PADDING:
-+ add_flags |= CPP_PADDING;
++ add_flags |= PREV_WHITE;
goto retry;
case CPP_NAME:
but it doesn't change anything on the outcome of those tests nor your
testcase above nor the C++ -std=c++98 -fpermissive test. CPP_PADDING
is 85 and PREV_WHITE 1, so it was setting PREV_WHITE too (just 3 other flags
too).
Jakub
Re: [PATCH] c: Handle scoped attributes in __has*attribute and scoped attribute parsing changes in -std=c11 etc. modes [PR114007]
On Thu, 22 Feb 2024, Jakub Jelinek wrote: > Hi! > > We aren't able to parse __has_attribute (vendor::attr) (and __has_c_attribute > and __has_cpp_attribute) in strict C < C23 modes. While in -std=gnu* modes > or in -std=c23 there is CPP_SCOPE token, in -std=c* (except for -std=c23) > there are is just a pair of CPP_COLON tokens. > The c-lex.cc hunk adds support for that. > > That leads to a question if we should return 1 or 0 from > __has_attribute (gnu::unused) or not, because while > [[gnu::unused]] is parsed fine in -std=gnu*/-std=c23 modes (sure, with > pedwarn for < C23), we do not parse it at all in -std=c* (except for > -std=c23), we only parse [[__extension__ gnu::unused]] there. While > the __extension__ in there helps to avoid the pedwarn, I think it is > better to be consistent between GNU and strict C < C23 modes and > parse [[gnu::unused]] too; on the other side, I think parsing > [[__extension__ gnu : : unused]] is too weird and undesirable. > > So, the following patch adds a flag during preprocessing at the point > where we normally create CPP_SCOPE tokens out of 2 consecutive colons > on the first CPP_COLON to mark the consecutive case (as we are tight > on the bits, I've reused the PURE_ZERO flag, which is used just by the > C++ FE and only ever set (both C and C++) on CPP_NUMBER tokens, this > new flag has the same value and is only ever used on CPP_COLON tokens) > and instead of checking loose_scope_p argument (i.e. whether it is > [[__extension__ ...]] or not), it just parses CPP_SCOPE or CPP_COLON > with CLONE_SCOPE flag followed by another CPP_COLON the same. > The latter will never appear in >= C23 or -std=gnu* modes, though > guarding its use say with flag_iso && !flag_isoc23 && doesn't really > work because the __extension__ case temporarily clears flag_iso flag. This patch (the one using COLON_SCOPE, *not* the one using PREV_WHITE) is OK. PREV_WHITE is about whether there is whitespace between the tokens in the macro expansion, for the purposes of stringization - I don't think it's appropriate to use here. For example, given #define COLON() : then [[gnu COLON()COLON() unused]] int x; should preferably not be valid; stringizing the results of expanding COLON()COLON() will produce "::" (PREV_WHITE not set), but that wouldn't be a valid attribute in C23 and I don't think anyone could reasonably expect it to be valid with previous standard versions. -- Joseph S. Myers josmy...@redhat.com
Re: [PATCH] c: Handle scoped attributes in __has*attribute and scoped attribute parsing changes in -std=c11 etc. modes [PR114007]
Hi, On Thu, 22 Feb 2024, Jakub Jelinek wrote: > > Hmm, shouldn't you be able to use (nonexistence of) the PREV_WHITE flag on > > the second COLON token to see that it's indeed a '::' without intervening > > whitespace? Instead of setting a new flag on the first COLON token? > > > > I.e. something like this: > > > >if (c_parser_next_token_is (parser, CPP_SCOPE) > > - || (loose_scope_p > > - && c_parser_next_token_is (parser, CPP_COLON) > > && c_parser_peek_2nd_token (parser)->type == CPP_COLON)) > > + && !(c_parser_peek_2nd_token (parser)->flags & PREV_WHITE))) > > > > ? > > That doesn't seem to work. Too bad then. I had hoped it would make the code easier without changes to c-lex. Well, then ... was worth a try, I'll crouch back under my stone :) Ciao, Michael.
Re: [PATCH] c: Handle scoped attributes in __has*attribute and scoped attribute parsing changes in -std=c11 etc. modes [PR114007]
On Thu, Feb 22, 2024 at 03:59:31PM +0100, Michael Matz wrote:
> Hello,
>
> On Thu, 22 Feb 2024, Jakub Jelinek wrote:
>
> > So, the following patch adds a flag during preprocessing at the point
> > where we normally create CPP_SCOPE tokens out of 2 consecutive colons
> > on the first CPP_COLON to mark the consecutive case (as we are tight
> > on the bits, I've reused the PURE_ZERO flag, which is used just by the
> > C++ FE and only ever set (both C and C++) on CPP_NUMBER tokens, this
> > new flag has the same value and is only ever used on CPP_COLON tokens)
>
> Hmm, shouldn't you be able to use (nonexistence of) the PREV_WHITE flag on
> the second COLON token to see that it's indeed a '::' without intervening
> whitespace? Instead of setting a new flag on the first COLON token?
>
> I.e. something like this:
>
>if (c_parser_next_token_is (parser, CPP_SCOPE)
> - || (loose_scope_p
> - && c_parser_next_token_is (parser, CPP_COLON)
> && c_parser_peek_2nd_token (parser)->type == CPP_COLON))
> + && !(c_parser_peek_2nd_token (parser)->flags & PREV_WHITE)))
>
> ?
That doesn't seem to work.
Compared to the posted patch it doesn't raise the 2 extra errors on
gcc.dg/c23-attr-syntax-6.c
#define JOIN2(A, B) A##B
typedef int [[__extension__ gnu JOIN2(:,:) vector_size (4)]] b5;
and that is just fine, that is error recovery after another error,
but doesn't diagnose:
#define BAR :
typedef int [[__extension__ gnu BAR BAR vector_size (4)]] b8;
nor
#define JOIN(A, B) A/**/B
typedef int [[__extension__ gnu JOIN(:,:) vector_size (4)]] b10;
(nor similar cases without __extension__).
Maybe it is about whether there are CPP_PADDING tokens in between if
PREV_WHITE is missing, but on c_parser_peek*_token we don't know if
there were any. Sure, on the c_common_has_attribute side that could
be done just by dropping the second loop.
--- gcc/doc/extend.texi.jj 2024-02-22 10:10:18.907029080 +0100
+++ gcc/doc/extend.texi 2024-02-22 16:06:33.197555930 +0100
@@ -12626,10 +12626,7 @@ In C, writing:
@end smallexample
suppresses warnings about using @samp{[[]]} attributes in C versions
-that predate C23@. Since the scope token @samp{::} is not a single
-lexing token in earlier versions of C, this construct also allows two colons
-to be used in place of @code{::}. GCC does not check whether the two
-colons are immediately adjacent.
+that predate C23@.
@end itemize
@code{__extension__} has no effect aside from this.
--- gcc/c-family/c-lex.cc.jj2024-02-22 10:09:48.408450163 +0100
+++ gcc/c-family/c-lex.cc 2024-02-22 16:09:50.822825035 +0100
@@ -357,7 +357,29 @@ c_common_has_attribute (cpp_reader *pfil
do
nxt_token = cpp_peek_token (pfile, idx++);
while (nxt_token->type == CPP_PADDING);
- if (nxt_token->type == CPP_SCOPE)
+ if (!c_dialect_cxx ()
+ && flag_iso
+ && !flag_isoc23
+ && nxt_token->type == CPP_COLON)
+ {
+ const cpp_token *prev_token = nxt_token;
+ do
+ nxt_token = cpp_peek_token (pfile, idx++);
+ while (nxt_token->type == CPP_PADDING);
+ if (nxt_token->type == CPP_COLON
+ && (nxt_token->flags & PREV_WHITE) == 0)
+ {
+ /* __has_attribute (vendor::attr) in -std=c17 etc. modes.
+:: isn't CPP_SCOPE but 2 CPP_COLON tokens, where the
+second one shouldn't have PREV_WHITE flag to distinguish
+it from : :. */
+ have_scope = true;
+ get_token_no_padding (pfile); // Eat first colon.
+ }
+ else
+ nxt_token = prev_token;
+ }
+ if (nxt_token->type == CPP_SCOPE || have_scope)
{
have_scope = true;
get_token_no_padding (pfile); // Eat scope.
--- gcc/c/c-parser.cc.jj2024-02-22 10:09:48.467449349 +0100
+++ gcc/c/c-parser.cc 2024-02-22 16:11:05.320795586 +0100
@@ -5705,8 +5705,7 @@ c_parser_omp_sequence_args (c_parser *pa
indicates whether this relaxation is in effect. */
static tree
-c_parser_std_attribute (c_parser *parser, bool for_tm,
- bool loose_scope_p = false)
+c_parser_std_attribute (c_parser *parser, bool for_tm)
{
c_token *token = c_parser_peek_token (parser);
tree ns, name, attribute;
@@ -5720,9 +5719,10 @@ c_parser_std_attribute (c_parser *parser
name = canonicalize_attr_name (token->value);
c_parser_consume_token (parser);
if (c_parser_next_token_is (parser, CPP_SCOPE)
- || (loose_scope_p
+ || (!flag_isoc23
&& c_parser_next_token_is (parser, CPP_COLON)
- && c_parser_peek_2nd_token (parser)->type == CPP_COLON))
+ && c_parser_peek_2nd_token (parser)->type == CPP_COLON
+ && (c_parser_peek_2nd_token (parser)->flags & PREV_WHITE) == 0))
{
ns = name;
if (c_parser_next_token_is (parser, CPP_COLON))
@@ -5841,8 +5841,7 @@ c_parser_std_attribute (c_parser *parser
}
static tree
-c_parser_s
Re: [PATCH] c: Handle scoped attributes in __has*attribute and scoped attribute parsing changes in -std=c11 etc. modes [PR114007]
Hello, On Thu, 22 Feb 2024, Jakub Jelinek wrote: > So, the following patch adds a flag during preprocessing at the point > where we normally create CPP_SCOPE tokens out of 2 consecutive colons > on the first CPP_COLON to mark the consecutive case (as we are tight > on the bits, I've reused the PURE_ZERO flag, which is used just by the > C++ FE and only ever set (both C and C++) on CPP_NUMBER tokens, this > new flag has the same value and is only ever used on CPP_COLON tokens) Hmm, shouldn't you be able to use (nonexistence of) the PREV_WHITE flag on the second COLON token to see that it's indeed a '::' without intervening whitespace? Instead of setting a new flag on the first COLON token? I.e. something like this: if (c_parser_next_token_is (parser, CPP_SCOPE) - || (loose_scope_p - && c_parser_next_token_is (parser, CPP_COLON) && c_parser_peek_2nd_token (parser)->type == CPP_COLON)) + && !(c_parser_peek_2nd_token (parser)->flags & PREV_WHITE))) ? Ciao, Michael.
