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? Marek