[PATCH 3/5] Atomic type qualifier - front end changes
Joseph S. Myers
joseph@codesourcery.com
Fri Aug 9 00:06:00 GMT 2013
Observations on this patch:
* build_qualified_type sets the qualifiers to exactly the set specified.
Thus, it looks like your handle_atomic_attribute will remove existing
qualifiers when adding "atomic".
* c-aux-info.c is meant to be generating actual valid C declarations, I
think, meaning _Atomic rather than "atomic".
* The code you have checking for _Atomic with function declarators appears
to be in the wrong place. What you're testing is the combination of
(_Atomic appears in declaration specifiers, as given by atomicp) and (some
declarator in the nested sequence of declarators is a function
declarator). But there are valid cases for this - for example a function
returning a pointer to atomic. And there are invalid cases involving an
atomic-qualified function type that you don't catch there - for example,
if _Atomic is applied to a typedef for a function type rather than
together with the declarator.
The relevant issue is whether the *function type* itself gets qualified.
There are already pedwarns-if-pedantic for this, given that there's
undefined behavior in ISO C for qualifiers on function types in general,
but making this case a hard error seems reasonable enough. To do that,
you need to adjust the cases
case cdk_pointer:
{
/* Merge any constancy or volatility into the target type
for the pointer. */
if (pedantic && TREE_CODE (type) == FUNCTION_TYPE
&& type_quals)
pedwarn (loc, OPT_Wpedantic,
"ISO C forbids qualified function types");
and similar for typedefs, type names, parameters and actual function
declarations, to give such errors. And similarly, for all the various
cases of things that can be declared, whenever qualifiers get applied in
grokdeclarator you need to ensure an error is given if the type is an
array type, since that is also a constraint violation in ISO C.
* The pedwarn for _Atomic outside C11 mode should I think only be a
pedwarn-if-pedantic, so pass OPT_Wpedantic instead of 0. And the text
should match other such pedwarns (i.e. "ISO C90 does not support _Atomic"
or "ISO C99 does not support _Atomic", depending on the standard version
selected).
* The parser comments with syntax should refer to _Atomic not "atomic".
* You change convert_for_assignment regarding discarding qualifiers.
Doing so is correct as far as it goes - not because of the reason in your
comment, but because in C11 terms _Atomic isn't a type qualifier. But I
don't think the change is enough. Because it's inside a conditional
checking for allowed cases: either side being a pointer to a void type, or
the target types being compatible. Now the rule in 6.5.16.1#1 refers to
"qualified or unqualified version of void", which in ISO C terms does not
include _Atomic void, so those checks need to disallow _Atomic void. And
similarly, it's not enough for comp_target_types to match the unqualified
types when it also needs to match whether _Atomic is specified (for that,
comp_target_types should probably be changed - conditional expressions
have the same issue).
The above relate specifically to what's in the patch. The listed issues
should of course have testcases added. I still need to review it on the
basis of reviews of C11 for references to qualifiers or _Atomic, to see
what might be missing from the patch, and on the basis of reviews of the C
front end for handling of qualifiers, to see what places might need to
handle _Atomic specially; I'll do those reviews separately. As previously
noted, one thing missing is the _Atomic ( type-name ) syntax.
--
Joseph S. Myers
joseph@codesourcery.com
More information about the Gcc-patches
mailing list