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.

(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.

Marek

Reply via email to