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

https://gcc.gnu.org/ml/gcc-patches/2017-10/msg00249.html
	gcc/cp/
	Warn on MVP declarations
	* cp-tree.h (struct cp_declarator): Add parenthesized field.
	* decl.c (grokdeclarator): Warn about unnecessary parens.
	* parser.c (make_declarator): Init parenthesized field.
	(cp_parser_direct_declarator): Set parenthesized field.

	gcc/
	* doc/invoke.texi (Wparentheses): Document C++ MVP behaviour.

	gcc/testsuite/
	* g++.dg/warn/mvp.C: New.

From-SVN: r253446

gcc/ChangeLog

@@ -1,3 +1,7 @@
+2017-10-05  Nathan Sidwell  <nathan@acm.org>
+
+	* doc/invoke.texi (Wparentheses): Document C++ MVP behaviour.
+
 2017-10-05  Tamar Christina  <tamar.christina@arm.com>
 
 	* config/arm/arm.c (arm_test_fpu_data): New.

gcc/cp/ChangeLog

@@ -1,5 +1,11 @@
 2017-10-05  Nathan Sidwell  <nathan@acm.org>
 
+	Warn on MVP declarations
+	* cp-tree.h (struct cp_declarator): Add parenthesized field.
+	* decl.c (grokdeclarator): Warn about unnecessary parens.
+	* parser.c (make_declarator): Init parenthesized field.
+	(cp_parser_direct_declarator): Set parenthesized field.
+
 	Kill IDENTIFIER_GLOBAL_VALUE, SET_IDENTIFIER_GLOBAL_VALUE
 	* cp-tree.h (IDENTIFIER_GLOBAL_VALUE,
 	SET_IDENTIFIER_GLOBAL_VALUE): Delete.

gcc/cp/cp-tree.h

@@ -5671,6 +5671,10 @@ struct cp_declarator {
   /* Whether we parsed an ellipsis (`...') just before the declarator,
      to indicate this is a parameter pack.  */
   BOOL_BITFIELD parameter_pack_p : 1;
+  /* If this declarator is parenthesized, this the open-paren.  It is
+     UNKNOWN_LOCATION when not parenthesized.  */
+  location_t parenthesized;
+
   location_t id_loc; /* Currently only set for cdk_id, cdk_decomp and
 			cdk_function. */
   /* GNU Attributes that apply to this declarator.  If the declarator

gcc/cp/decl.c

@@ -10807,6 +10807,13 @@ grokdeclarator (const cp_declarator *declarator,
 					    attr_flags);
 	}
 
+      /* We don't want to warn in parmeter context because we don't
+	 yet know if the parse will succeed, and this might turn out
+	 to be a constructor call.  */
+      if (decl_context != PARM
+	  && declarator->parenthesized != UNKNOWN_LOCATION)
+	warning_at (declarator->parenthesized, OPT_Wparentheses,
+		    "unnecessary parentheses in declaration of %qs", name);
       if (declarator->kind == cdk_id || declarator->kind == cdk_decomp)
 	break;
 

gcc/cp/parser.c

@@ -1451,6 +1451,7 @@ make_declarator (cp_declarator_kind kind)
 
   declarator = (cp_declarator *) alloc_declarator (sizeof (cp_declarator));
   declarator->kind = kind;
+  declarator->parenthesized = UNKNOWN_LOCATION;
   declarator->attributes = NULL_TREE;
   declarator->std_attributes = NULL_TREE;
   declarator->declarator = NULL;
@@ -19808,6 +19809,7 @@ cp_parser_direct_declarator (cp_parser* parser,
   bool saved_in_declarator_p = parser->in_declarator_p;
   bool first = true;
   tree pushed_scope = NULL_TREE;
+  cp_token *open_paren = NULL, *close_paren = NULL;
 
   while (true)
     {
@@ -19858,6 +19860,8 @@ cp_parser_direct_declarator (cp_parser* parser,
 	      tree params;
 	      bool is_declarator = false;
 
+	      open_paren = NULL;
+
 	      /* In a member-declarator, the only valid interpretation
 		 of a parenthesis is the start of a
 		 parameter-declaration-clause.  (It is invalid to
@@ -19979,6 +19983,7 @@ cp_parser_direct_declarator (cp_parser* parser,
 	      parser->default_arg_ok_p = saved_default_arg_ok_p;
 	      parser->in_declarator_p = saved_in_declarator_p;
 
+	      open_paren = token;
 	      /* Consume the `('.  */
 	      matching_parens parens;
 	      parens.consume_open (parser);
@@ -19992,6 +19997,7 @@ cp_parser_direct_declarator (cp_parser* parser,
 	      parser->in_type_id_in_expr_p = saved_in_type_id_in_expr_p;
 	      first = false;
 	      /* Expect a `)'.  */
+	      close_paren = cp_lexer_peek_token (parser->lexer);
 	      if (!parens.require_close (parser))
 		declarator = cp_error_declarator;
 	      if (declarator == cp_error_declarator)
@@ -20013,6 +20019,7 @@ cp_parser_direct_declarator (cp_parser* parser,
 	  if (ctor_dtor_or_conv_p)
 	    *ctor_dtor_or_conv_p = 0;
 
+	  open_paren = NULL;
 	  first = false;
 	  parser->default_arg_ok_p = false;
 	  parser->in_declarator_p = true;
@@ -20308,6 +20315,22 @@ cp_parser_direct_declarator (cp_parser* parser,
      point.  That's an error; the declarator is not optional.  */
   if (!declarator)
     cp_parser_error (parser, "expected declarator");
+  else if (open_paren)
+    {
+      /* Record overly parenthesized declarator so we can give a
+	 diagnostic about confusing decl/expr disambiguation.  */
+      if (declarator->kind == cdk_array)
+	{
+	  /* If the open and close parens are on different lines, this
+	     is probably a formatting thing, so ignore.  */
+	  expanded_location open = expand_location (open_paren->location);
+	  expanded_location close = expand_location (close_paren->location);
+	  if (open.line != close.line || open.file != close.file)
+	    open_paren = NULL;
+	}
+      if (open_paren)
+	declarator->parenthesized = open_paren->location;
+    }
 
   /* If we entered a scope, we must exit it now.  */
   if (pushed_scope)

gcc/doc/invoke.texi

@@ -4579,6 +4579,17 @@ in the @code{?}: operator is a boolean expression, the omitted value is
 always 1.  Often programmers expect it to be a value computed
 inside the conditional expression instead.
 
+For C++ this also warns for some cases of unnecessary parentheses in
+declarations, which can indicate an attempt at a function call instead
+of a declaration:
+@smallexample
+@{
+  // Declares a local variable called mymutex.
+  std::unique_lock<std::mutex> (mymutex);
+  // User meant std::unique_lock<std::mutex> lock (mymutex);
+@}
+@end smallexample
+
 This warning is enabled by @option{-Wall}.
 
 @item -Wsequence-point

gcc/testsuite/ChangeLog

@@ -1,3 +1,7 @@
+2017-10-05  Nathan Sidwell  <nathan@acm.org>
+
+	* g++.dg/warn/mvp.C: New.
+
 2017-10-05  Jakub Jelinek  <jakub@redhat.com>
 
 	* gcc.dg/gomp/pr82374.c (SIZE): Change from 1G to 1M to make it ilp32

gcc/testsuite/g++.dg/warn/mvp.C

@@ -0,0 +1,78 @@
+// { dg-additional-options -Wparentheses }
+
+// Most Vexing Parse warnings
+// in C++ anythig that syntactically looks like a decl IS a decl, this
+// can lead to confused users, but worse silent unexpectedly unsafe
+// code generation.
+
+int (a); // { dg-warning "" }
+int (*b);  // { dg-warning "" }
+extern int (&c);  // { dg-warning "" }
+
+int h1 = 0, h2 = 0;
+struct H { H(...);};
+
+namespace fns 
+{
+  int (*a) ();
+  int (b) ();
+  int (*c ()) ();
+  int (d1 ()); // { dg-warning "" }
+  int (d2 // { dg-warning "" }
+       ());
+  int (e) (int);
+  int g (int (a)); // No warning because ...
+  H h (int (h1), int (h2), 3); // ... not a function decl.
+}
+
+namespace arys
+{
+  int (*a)[1];
+  int (b)[1];
+  int (*c[1])[1];
+  int (d1[1]); // { dg-warning "" }
+  int (d2
+       [1]);
+  int (e[1])[1];
+}
+
+namespace complex
+{
+  int (*a())[1];
+  int (*b[1])();
+  int ((*c1())[1]); // { dg-warning "" }
+  int ((*c2())
+       [1]);
+  int ((*d1[1])()); // { dg-warning "" }
+  int ((*d2[1])	 // { dg-warning "" }
+       ());
+}
+
+namespace motivation
+{
+  typedef int shared_mutex; // for exposition
+  struct locker
+  {
+    locker ();
+    locker (int &r);
+    ~locker ();
+  };
+  class protected_state 
+  {
+    shared_mutex mutex; // not a real mutex type
+    int state;
+
+  public:
+    void not_thread_safe ()
+    {
+      locker (mutex); // { dg-warning "" }
+      state++; // oops
+    }
+    
+    void thread_safe ()
+    {
+      locker lock (mutex);
+      state++; // ok;
+    }
+  };
+}