Re: [PATCH] C++ warning on vexing parse

[Jason Merrill <jason at redhat dot com>]

On Tue, Oct 3, 2017 at 1:08 PM, Nathan Sidwell <nathan@acm.org> wrote:
> [non-c++ people on CC, there's a reason ...]
>
> This patch adds a new warning, concerning unnecessary parentheses on a
> declaration.  For instance:
>    prettyprinter (pp);
> That's a declaration of a pp variable of type prettyprinter -- not a call of
> a prettyprinter function.  The reason is that in C++, anything that is
> syntactically a declaration, is a declaration.  This can lead to surprising
> unexpected code generation, and the documentation I added provides
> motivation:
>   std::unique_lock<std::mutex> (mymutex);
> That is NOT a lock of mymutex.  It's a declaration of a variable called
> mymutex locking no mutex.  If we're in a member function and 'mymutex' is a
> field, Wshadow-parm doesn't trigger either.
>
> This patch notes when a direct-declarator is parenthesized, and then warns
> about it in grokdeclarator.  Obviously, parens are sometimes necessary --
> when declaring pointers to arrays or functions, and we don't warn then.  The
> simplest way to do that was during the parsing, not in grokdeclarator, where
> the cp_declarator object is constant.  We'd either have to change that, or
> have some other local state.
>
> I added this to -Wparentheses (enabled by -Wall).  It triggered in a few
> cases during bootstrap:
>
> 1) in toplev.c we have the above prettyprinter example.  I think that's
> clearly poorly formatted code.

Definitely.

> 2) caller-save.c: insert_save has 'HARD_REG_SET (*to_save)', which also
> seems poorly formatted to me -- other cases of ptr-to-HRS don't do this.

Probably autopilot from someone used to parenthesizing declarators for
pointer-to-function or pointer-to-array types.

> 3) A couple of places do:
>    T (name  // line break to avoid wrap
>       [LONGEXPR][OTHERLONGEXPR]);
>
> The parens aid the editor's formatter. The patch removes the parens, but I
> can see there may be disagreement.  I suppose we could permit parens at the
> outermost declarator for an array?
> Affects ira-int.h (target_ira_int::x_ira_reg_mode_hard_regset)
>          reload.h (target_reload::x_regno_save_mode)

We could also check whether the declarator spans lines to detect this case.

> 4) A set of typedef'd function types (NOT ptr-to-fn).  The name being
> typedef'd is parenthesized and not an idiom I recall seeing before. AFAICT
> these are the only cases of typedefing a plain fn.  We could, I suppose,
> ignore parens on a typedef'd fntype, but that seems a little random.
> Affects gengtype.c (frul_actionrout_t)
>         lto-streamer.h (lto_get_section_data_f & to_free_section_data_f)
>         tree-ssa-threadedge.c (pfn_simplify)

Hmm, I don't think we want to diagnose these; if a parameter-list
follows the parenthesized declarator, it isn't ambiguous.

3 and 4 seem like false positives.  The problematic cases are all ones
where the parenthesized name is at the end of the declarator; if the
declarator continues after the name, either within or without the
parens, this

> Jason, do you have concerns about the C++ patch itself?

Nope.

Jason