On 10/29/20 2:11 PM, Marek Polacek wrote:
On Thu, Oct 29, 2020 at 11:17:37AM -0400, Jason Merrill via Gcc-patches wrote:
On 10/28/20 7:40 PM, Marek Polacek wrote:
On Wed, Oct 28, 2020 at 03:09:08PM -0400, Jason Merrill wrote:
On 10/28/20 1:58 PM, Marek Polacek wrote:
On Wed, Oct 28, 2020 at 01:26:53AM -0400, Jason Merrill via Gcc-patches wrote:
On 10/24/20 7:40 PM, Marek Polacek wrote:
On Fri, Oct 23, 2020 at 09:33:38PM -0400, Jason Merrill via Gcc-patches wrote:
On 10/23/20 3:01 PM, Marek Polacek wrote:
This patch implements the -Wvexing-parse warning to warn about the
sneaky most vexing parse rule in C++: the cases when a declaration
looks like a variable definition, but the C++ language requires it
to be interpreted as a function declaration. This warning is on by
default (like clang++). From the docs:
void f(double a) {
int i(); // extern int i (void);
int n(int(a)); // extern int n (int);
}
Another example:
struct S { S(int); };
void f(double a) {
S x(int(a)); // extern struct S x (int);
S y(int()); // extern struct S y (int (*) (void));
S z(); // extern struct S z (void);
}
You can find more on this in [dcl.ambig.res].
I spent a fair amount of time on fix-it hints so that GCC can recommend
various ways to resolve such an ambiguity. Sometimes that's tricky.
E.g., suggesting default-initialization when the class doesn't have
a default constructor would not be optimal. Suggesting {}-init is also
not trivial because it can use an initializer-list constructor if no
default constructor is available (which ()-init wouldn't do). And of
course, pre-C++11, we shouldn't be recommending {}-init at all.
What do you think of, instead of passing the type down into the declarator
parse, adding the paren locations to cp_declarator::function and giving the
diagnostic from cp_parser_init_declarator instead?
Oops, now I see there's already cp_declarator::parenthesized; might as well
reuse that. And maybe change it to a range, while we're at it.
I'm afraid I can't reuse it because grokdeclarator uses it to warn about
"unnecessary parentheses in declaration". So when we have:
int (x());
declarator->parenthesized points to the outer parens (if any), whereas
declarator->u.function.parens_loc should point to the inner ones. We also
have declarator->id_loc but I think we should only use it for declarator-ids.
Makes sense.
(We should still adjust ->parenthesized to be a range to generate a better
diagnostic; I shall send a patch soon.)
Hmm, I wonder why we have the parenthesized_p parameter to some of these
functions, since we can look at the declarator to find that information...
That would be a nice cleanup.
Interesting idea. I suppose it's better, and makes the implementation
more localized. The approach here is that if the .function.parens_loc
is UNKNOWN_LOCATION, we've not seen a vexing parse.
I'd rather always set the parens location, and then analyze the
cp_declarator in warn_about_ambiguous_parse to see if it's a vexing parse;
we should have all the information we need.
I could always set .parens_loc, but then I'd still need another flag telling
me whether we had an ambiguity. Otherwise I don't know how I would tell
apart e.g. "int f()" (warn) v. "int f(void)" (don't warn), etc.
Ah, I was thinking that we still had the parameter declarators, but now I
see that cp_parser_parameter_declaration_list groks them and returns a
TREE_LIST. We could set a TREE_LANG_FLAG on each TREE_LIST if its parameter
declarator was parenthesized?
I think so, looks like we have a bunch of free TREE_LANG_FLAG slots on
a TREE_LIST. But cp_parser_parameter_declaration_clause can return
a void_list_node, so I assume I'd have to copy_node it before setting
some new flag in it. Do you think that'd be fine?
There's no declarator in a void_list_node, so we shouldn't need to set a
"declarator is parenthesized" flag on it.
I guess I'm still not clear on how I would distinguish between
int f() and int f(void). When I look at the cdk_function declarator,
all I can see is the .parameters TREE_LIST, which for both cases will
be the same void_list_node, but we should only warn for the former.
What am I missing?
I'm just being dense. You're right that we would need to distinguish
those two. Perhaps an explicit_void_parms_node or something like that
for during parsing; it looks like grokparms will turn it into
void_list_node as other code expects.
Jason
