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

[Eric Gallager <egall at gwmail dot gwu dot edu>]

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.
>
> 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.
>
> 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)
>
> 3) 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)
>
> If you've objections to the changes I've made in instances of #3 & #4 let me
> know.
>
> Jason, do you have concerns about the C++ patch itself?
>
> nathan
> --
> Nathan Sidwell

Could you check and see if this fixes any of the preexisting
currently-open bugs related to most-vexing-parse (or similar) warnings
on Bugzilla? For example:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64679
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25814
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69855