[C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 1140 bytes --]

Hi,

thus I had to revert my first try, when it caused c++/85713. I added two 
testcases for the latter (the second one covering what I learned from 
yet another defective try which I attached to the trail of c++/84588 
yesterday) and finally figured out that the problem was that I was 
incorrectly calling abort_fully_implicit_template while tentatively 
parsing (if you look at the new lambda-generic-85713.C, that made 
impossible correctly parsing 'auto (&array) [5]' as second lambda 
parameter).

Anyway, a few days ago, while looking for a completely different 
solution and comparing to other compilers too, I noticed that, more 
generally, we were missing a check in cp_parser_condition that we aren't 
declaring a function type when we are sure that we are handling a 
declaration: simply adding such a check covers as a special case 
c++/84588 too, and, being the check in cp_parser_condition, it 
automatically covers variants for conditions elsewhere, eg, for, while 
loops: with the patchlet below we handle all of them very similarly to 
clang and icc.

Tested x86_64-linux.

Thanks, Paolo.

//////////////////////////////


[-- Attachment #2: CL_84588_4 --]
[-- Type: text/plain, Size: 305 bytes --]

/cp
2018-05-17  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/84588
	* parser.c (cp_parser_condition): Reject a declaration of
	a function type.

/testsuite
2018-05-17  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/84588
	* g++.dg/cpp1y/pr84588.C: New.
	* g++.old-deja/g++.jason/cond.C: Adjust.

[-- Attachment #3: patch_84588_4 --]
[-- Type: text/plain, Size: 2858 bytes --]

Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 260308)
+++ cp/parser.c	(working copy)
@@ -11571,6 +11571,7 @@ cp_parser_condition (cp_parser* parser)
       tree attributes;
       cp_declarator *declarator;
       tree initializer = NULL_TREE;
+      location_t loc = cp_lexer_peek_token (parser->lexer)->location;
 
       /* Parse the declarator.  */
       declarator = cp_parser_declarator (parser, CP_PARSER_DECLARATOR_NAMED,
@@ -11597,15 +11598,23 @@ cp_parser_condition (cp_parser* parser)
 	 for sure.  */
       if (cp_parser_parse_definitely (parser))
 	{
-	  tree pushed_scope;
+	  tree pushed_scope = NULL_TREE;
 	  bool non_constant_p;
 	  int flags = LOOKUP_ONLYCONVERTING;
 
 	  /* Create the declaration.  */
-	  decl = start_decl (declarator, &type_specifiers,
-			     /*initialized_p=*/true,
-			     attributes, /*prefix_attributes=*/NULL_TREE,
-			     &pushed_scope);
+	  if (declarator->kind == cdk_function)
+	    {
+	      error_at (loc, "a function type is not allowed here");
+	      if (parser->fully_implicit_function_template_p)
+	      	abort_fully_implicit_template (parser);
+	      decl = error_mark_node;
+	    }
+	  else
+	    decl = start_decl (declarator, &type_specifiers,
+			       /*initialized_p=*/true,
+			       attributes, /*prefix_attributes=*/NULL_TREE,
+			       &pushed_scope);
 
 	  /* Parse the initializer.  */
 	  if (cp_lexer_next_token_is (parser->lexer, CPP_OPEN_BRACE))
Index: testsuite/g++.dg/cpp1y/pr84588.C
===================================================================
--- testsuite/g++.dg/cpp1y/pr84588.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/pr84588.C	(working copy)
@@ -0,0 +1,25 @@
+// { dg-do compile { target c++14 } }
+
+struct a {
+  void b() {}
+  void c(void (*) () = [] {
+      if (a a(int auto) {})  // { dg-error "two or more data types|function type" }
+      ;
+  }) {}
+};
+
+struct d {
+  void e() {}
+  void f(void (*) () = [] {
+      for (;d d(int auto) {};)  // { dg-error "two or more data types|function type" }
+      ;
+  }) {}
+};
+
+struct g {
+  void h() {}
+  void i(void (*) () = [] {
+      while (g g(int auto) {})  // { dg-error "two or more data types|function type" }
+      ;
+  }) {}
+};
Index: testsuite/g++.old-deja/g++.jason/cond.C
===================================================================
--- testsuite/g++.old-deja/g++.jason/cond.C	(revision 260308)
+++ testsuite/g++.old-deja/g++.jason/cond.C	(working copy)
@@ -47,8 +47,7 @@ int main()
   if (struct B * foo = new B)
     ;
 
-  if (int f () = 1)		// { dg-warning "extern" "extern" } 
-  // { dg-error "is initialized like a variable" "var" { target *-*-* } .-1 }
+  if (int f () = 1)		// { dg-error "function type" } 
     ;
   
   if (int a[2] = {1, 2})	// { dg-error "extended init" "" { target { ! c++11 } } }

Re: [C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[Paolo Carlini]

PS: maybe better using function_declarator_p??? I think I regression 
tested that variant too, at some point.

Paolo.

Re: [C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[Jason Merrill]

On Thu, May 17, 2018 at 10:27 AM, Paolo Carlini
<paolo.carlini@oracle.com> wrote:
> PS: maybe better using function_declarator_p?

I think so, yes.  The relevant rule seems to be "The declarator shall
not specify a function or an array.", so let's check for arrays, too.

Jason

Re: [C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 823 bytes --]

Hi,

On 17/05/2018 16:58, Jason Merrill wrote:
> On Thu, May 17, 2018 at 10:27 AM, Paolo Carlini
> <paolo.carlini@oracle.com> wrote:
>> PS: maybe better using function_declarator_p?
> I think so, yes.  The relevant rule seems to be "The declarator shall
> not specify a function or an array.", so let's check for arrays, too.
Agreed. I had the amended patch ready when I noticed (again) that it 
wasn't addressing another related class of issues which involves 
declarators not followed by initializers. Thus I tried to fix those too, 
and the below which moves the check up appears to work fine, passes 
testing, etc. Are there any risks that an erroneous function / array as 
declarator is in fact a well formed expression?!? I haven't been able so 
far to construct examples...

Thanks!
Paolo.

////////////////////////

[-- Attachment #2: patch_84588_7 --]
[-- Type: text/plain, Size: 5794 bytes --]

Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 260331)
+++ cp/parser.c	(working copy)
@@ -11527,6 +11527,33 @@ cp_parser_selection_statement (cp_parser* parser,
     }
 }
 
+/* Helper function for cp_parser_condition.  Enforces [stmt.stmt]:
+   The declarator shall not specify a function or an array.  Returns
+   TRUE is the declator is valid, FALSE otherwise.  */
+
+static bool
+cp_parser_check_condition_declarator (cp_parser* parser,
+				      cp_declarator *declarator,
+				      location_t loc)
+{
+  if (function_declarator_p (declarator)
+      || declarator->kind == cdk_array)
+    {
+      if (declarator->kind == cdk_array)
+	error_at (loc, "an array type is not allowed here");
+      else
+	error_at (loc, "a function type is not allowed here");
+      if (parser->fully_implicit_function_template_p)
+	abort_fully_implicit_template (parser);
+      cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true,
+					     /*or_comma=*/false,
+					     /*consume_paren=*/false);
+      return false;
+    }
+  else
+    return true;
+}
+
 /* Parse a condition.
 
    condition:
@@ -11571,6 +11598,7 @@ cp_parser_condition (cp_parser* parser)
       tree attributes;
       cp_declarator *declarator;
       tree initializer = NULL_TREE;
+      location_t loc = cp_lexer_peek_token (parser->lexer)->location;
 
       /* Parse the declarator.  */
       declarator = cp_parser_declarator (parser, CP_PARSER_DECLARATOR_NAMED,
@@ -11592,10 +11620,15 @@ cp_parser_condition (cp_parser* parser)
       if (cp_lexer_next_token_is_not (parser->lexer, CPP_EQ)
 	  && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_BRACE))
 	cp_parser_simulate_error (parser);
-	
+
       /* If we did see an `=' or '{', then we are looking at a declaration
 	 for sure.  */
-      if (cp_parser_parse_definitely (parser))
+      bool decl_p = cp_parser_parse_definitely (parser);
+
+      if (!cp_parser_check_condition_declarator (parser, declarator, loc))
+	return error_mark_node;
+
+      if (decl_p)
 	{
 	  tree pushed_scope;
 	  bool non_constant_p;
Index: testsuite/g++.dg/cpp0x/cond1.C
===================================================================
--- testsuite/g++.dg/cpp0x/cond1.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/cond1.C	(working copy)
@@ -0,0 +1,23 @@
+// PR c++/84588
+// { dg-do compile { target c++11 } }
+
+void foo()
+{
+  if (int bar() {})  // { dg-error "function type is not allowed" }
+    ;
+
+  for (;int bar() {};)  // { dg-error "function type is not allowed" }
+    ;
+
+  while (int bar() {})  // { dg-error "function type is not allowed" }
+    ;
+
+  if (int a[] {})  // { dg-error "array type is not allowed" }
+    ;
+
+  for (;int a[] {};)  // { dg-error "array type is not allowed" }
+    ;
+
+  while (int a[] {})  // { dg-error "array type is not allowed" }
+    ;
+}
Index: testsuite/g++.dg/cpp1y/pr84588-1.C
===================================================================
--- testsuite/g++.dg/cpp1y/pr84588-1.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/pr84588-1.C	(working copy)
@@ -0,0 +1,25 @@
+// { dg-do compile { target c++14 } }
+
+struct a {
+  void b() {}
+  void c(void (*) () = [] {
+      if (a a(int auto) {})  // { dg-error "two or more data types|function type" }
+      ;
+  }) {}
+};
+
+struct d {
+  void e() {}
+  void f(void (*) () = [] {
+      for (;d d(int auto) {};)  // { dg-error "two or more data types|function type" }
+      ;
+  }) {}
+};
+
+struct g {
+  void h() {}
+  void i(void (*) () = [] {
+      while (g g(int auto) {})  // { dg-error "two or more data types|function type" }
+      ;
+  }) {}
+};
Index: testsuite/g++.dg/cpp1y/pr84588-2.C
===================================================================
--- testsuite/g++.dg/cpp1y/pr84588-2.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/pr84588-2.C	(working copy)
@@ -0,0 +1,25 @@
+// { dg-do compile { target c++14 } }
+
+struct a {
+  void b() {}
+  void c(void (*) () = [] {
+      if (a a(int auto))  // { dg-error "two or more data types|function type" }
+      ;
+  }) {}
+};
+
+struct d {
+  void e() {}
+  void f(void (*) () = [] {
+      for (;d d(int auto);)  // { dg-error "two or more data types|function type" }
+      ;
+  }) {}
+};
+
+struct g {
+  void h() {}
+  void i(void (*) () = [] {
+      while (g g(int auto))  // { dg-error "two or more data types|function type" }
+      ;
+  }) {}
+};
Index: testsuite/g++.dg/parse/cond6.C
===================================================================
--- testsuite/g++.dg/parse/cond6.C	(nonexistent)
+++ testsuite/g++.dg/parse/cond6.C	(working copy)
@@ -0,0 +1,22 @@
+// PR c++/84588
+
+void foo()
+{
+  if (int bar())  // { dg-error "function type is not allowed" }
+    ;
+
+  for (;int bar();)  // { dg-error "function type is not allowed" }
+    ;
+
+  while (int bar())  // { dg-error "function type is not allowed" }
+    ;
+
+  if (int a[])  // { dg-error "array type is not allowed" }
+    ;
+
+  for (;int a[];)  // { dg-error "array type is not allowed" }
+    ;
+
+  while (int a[])  // { dg-error "array type is not allowed" }
+    ;
+}
Index: testsuite/g++.old-deja/g++.jason/cond.C
===================================================================
--- testsuite/g++.old-deja/g++.jason/cond.C	(revision 260331)
+++ testsuite/g++.old-deja/g++.jason/cond.C	(working copy)
@@ -47,11 +47,10 @@ int main()
   if (struct B * foo = new B)
     ;
 
-  if (int f () = 1)		// { dg-warning "extern" "extern" } 
-  // { dg-error "is initialized like a variable" "var" { target *-*-* } .-1 }
+  if (int f () = 1)		// { dg-error "function type" } 
     ;
   
-  if (int a[2] = {1, 2})	// { dg-error "extended init" "" { target { ! c++11 } } }
+  if (int a[2] = {1, 2})	// { dg-error "array type" }
     ;
 
 }

Re: [C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[Jason Merrill]

On Thu, May 17, 2018 at 5:54 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> On 17/05/2018 16:58, Jason Merrill wrote:
>>
>> On Thu, May 17, 2018 at 10:27 AM, Paolo Carlini
>> <paolo.carlini@oracle.com> wrote:
>>>
>>> PS: maybe better using function_declarator_p?
>>
>> I think so, yes.  The relevant rule seems to be "The declarator shall
>> not specify a function or an array.", so let's check for arrays, too.
>
> Agreed. I had the amended patch ready when I noticed (again) that it wasn't
> addressing another related class of issues which involves declarators not
> followed by initializers. Thus I tried to fix those too, and the below which
> moves the check up appears to work fine, passes testing, etc. Are there any
> risks that an erroneous function / array as declarator is in fact a well
> formed expression?!?

That doesn't matter; if it parses as a declarator, it's a declarator,
even if it's an ill-formed declarator.  But...

+      bool decl_p = cp_parser_parse_definitely (parser);
+      if (!cp_parser_check_condition_declarator (parser, declarator, loc))
+        return error_mark_node;

...if cp_parser_parse_definitely returns false, parsing as a
declarator failed, so we shouldn't look at "declarator".

Also, "here" in the diagnostic seems unnecessarily vague; we could be
more specific.  Maybe "condition declares a function/array"?

Jason

Re: [C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[Paolo Carlini]

Hi,

On 18/05/2018 01:21, Jason Merrill wrote:
> On Thu, May 17, 2018 at 5:54 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
>> On 17/05/2018 16:58, Jason Merrill wrote:
>>> On Thu, May 17, 2018 at 10:27 AM, Paolo Carlini
>>> <paolo.carlini@oracle.com> wrote:
>>>> PS: maybe better using function_declarator_p?
>>> I think so, yes.  The relevant rule seems to be "The declarator shall
>>> not specify a function or an array.", so let's check for arrays, too.
>> Agreed. I had the amended patch ready when I noticed (again) that it wasn't
>> addressing another related class of issues which involves declarators not
>> followed by initializers. Thus I tried to fix those too, and the below which
>> moves the check up appears to work fine, passes testing, etc. Are there any
>> risks that an erroneous function / array as declarator is in fact a well
>> formed expression?!?
> That doesn't matter; if it parses as a declarator, it's a declarator,
> even if it's an ill-formed declarator.  But...
>
> +      bool decl_p = cp_parser_parse_definitely (parser);
> +      if (!cp_parser_check_condition_declarator (parser, declarator, loc))
> +        return error_mark_node;
>
> ...if cp_parser_parse_definitely returns false, parsing as a
> declarator failed, so we shouldn't look at "declarator".
Uhm, then you are saying that we should fix cp_parser_declarator itself, 
right? Because we don't want cp_parser_parse_definitely returning false 
after cp_parser_declarator parses, say, 'if (int foo())' and therefore 
cp_parser_condition proceed with cp_parser_expression, we want to emit 
our error and bail out. Therefore the problem in the new patch seems 
that it tries to paper over that cp_parser_declarator issue in the 
caller?!? Like, Ok, cp_parser_declarator failed, but it was anyway 
trying to declare a function / array and that can't possibly be an 
expression, see what I mean? *Somehow*, the question you answered above.

Paolo.

Re: [C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[Paolo Carlini]

Hi again,

On 18/05/2018 02:31, Paolo Carlini wrote:
> Hi,
>
> On 18/05/2018 01:21, Jason Merrill wrote:
>> On Thu, May 17, 2018 at 5:54 PM, Paolo Carlini 
>> <paolo.carlini@oracle.com> wrote:
>>> On 17/05/2018 16:58, Jason Merrill wrote:
>>>> On Thu, May 17, 2018 at 10:27 AM, Paolo Carlini
>>>> <paolo.carlini@oracle.com> wrote:
>>>>> PS: maybe better using function_declarator_p?
>>>> I think so, yes.  The relevant rule seems to be "The declarator shall
>>>> not specify a function or an array.", so let's check for arrays, too.
>>> Agreed. I had the amended patch ready when I noticed (again) that it 
>>> wasn't
>>> addressing another related class of issues which involves 
>>> declarators not
>>> followed by initializers. Thus I tried to fix those too, and the 
>>> below which
>>> moves the check up appears to work fine, passes testing, etc. Are 
>>> there any
>>> risks that an erroneous function / array as declarator is in fact a 
>>> well
>>> formed expression?!?
>> That doesn't matter; if it parses as a declarator, it's a declarator,
>> even if it's an ill-formed declarator.  But...
>>
>> +      bool decl_p = cp_parser_parse_definitely (parser);
>> +      if (!cp_parser_check_condition_declarator (parser, declarator, 
>> loc))
>> +        return error_mark_node;
>>
>> ...if cp_parser_parse_definitely returns false, parsing as a
>> declarator failed, so we shouldn't look at "declarator".
> Uhm, then you are saying that we should fix cp_parser_declarator 
> itself, right? Because we don't want cp_parser_parse_definitely 
> returning false after cp_parser_declarator parses, say, 'if (int 
> foo())' and therefore cp_parser_condition proceed with 
> cp_parser_expression, we want to emit our error and bail out. 
> Therefore the problem in the new patch seems that it tries to paper 
> over that cp_parser_declarator issue in the caller?!? Like, Ok, 
> cp_parser_declarator failed, but it was anyway trying to declare a 
> function / array and that can't possibly be an expression, see what I 
> mean? *Somehow*, the question you answered above.
Ok, now I finally see what's the exact issue you pointed out (I'm a bit 
tired). Seems fixable.

If I understand correctly, the reason why the 3 lines you cited above 
are wrong as they are is that my patch *assumes* that 
cp_parser_declarator didn't really fail and cp_parser_condition has 
forced the tentative parse to fail by calling cp_parser_simulate_error 
immediately before when it didn't see an initializer immediately 
following. That's actually true for 'if (int foo())', thus it makes 
sense to check the declarator anyway for such cases *even* if 
cp_parser_parse_definitely returns false. See what I mean?

Therefore, it seems to me that an amended patch would rearrange 
cp_parser_condition to *not* call cp_parser_simulate_error for the cases 
we care about ('if (int foo())') and instead check the declarator.

I'll work on that tomorrow...

Thanks,
Paolo.

Re: [C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 1868 bytes --]

Hi,

>> On 18/05/2018 01:21, Jason Merrill wrote:
>>> On Thu, May 17, 2018 at 5:54 PM, Paolo Carlini 
>>> <paolo.carlini@oracle.com> wrote:
>>>> On 17/05/2018 16:58, Jason Merrill wrote:
>>>>> On Thu, May 17, 2018 at 10:27 AM, Paolo Carlini
>>>>> <paolo.carlini@oracle.com> wrote:
>>>>>> PS: maybe better using function_declarator_p?
>>>>> I think so, yes.  The relevant rule seems to be "The declarator shall
>>>>> not specify a function or an array.", so let's check for arrays, too.
>>>> Agreed. I had the amended patch ready when I noticed (again) that 
>>>> it wasn't
>>>> addressing another related class of issues which involves 
>>>> declarators not
>>>> followed by initializers. Thus I tried to fix those too, and the 
>>>> below which
>>>> moves the check up appears to work fine, passes testing, etc. Are 
>>>> there any
>>>> risks that an erroneous function / array as declarator is in fact a 
>>>> well
>>>> formed expression?!?
>>> That doesn't matter; if it parses as a declarator, it's a declarator,
>>> even if it's an ill-formed declarator.  But...
>>>
>>> +      bool decl_p = cp_parser_parse_definitely (parser);
>>> +      if (!cp_parser_check_condition_declarator (parser, 
>>> declarator, loc))
>>> +        return error_mark_node;
>>>
>>> ...if cp_parser_parse_definitely returns false, parsing as a
>>> declarator failed, so we shouldn't look at "declarator".
What I'm attaching below isn't affected by this problem: I moved the 
check further up - *before* maybe calling cp_parser_simulated_error 
because an initializer isn't in sight - and is carried out only when 
!cp_parser_error_occurred, thus cp_parser_declarator succeeded . 
cp_parser_commit_to_tentative_parse is called when the check fails. 
Bootstrapped and tested on x86_64-linux.

Thanks!
Paolo.

//////////////////////

[-- Attachment #2: patch_84588_8 --]
[-- Type: text/plain, Size: 5851 bytes --]

Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 260347)
+++ cp/parser.c	(working copy)
@@ -11527,6 +11527,34 @@ cp_parser_selection_statement (cp_parser* parser,
     }
 }
 
+/* Helper function for cp_parser_condition.  Enforces [stmt.stmt]/2:
+   The declarator shall not specify a function or an array.  Returns
+   TRUE if the declarator is valid, FALSE otherwise.  */
+
+static bool
+cp_parser_check_condition_declarator (cp_parser* parser,
+				      cp_declarator *declarator,
+				      location_t loc)
+{
+  if (function_declarator_p (declarator)
+      || declarator->kind == cdk_array)
+    {
+      cp_parser_commit_to_tentative_parse (parser);
+      if (declarator->kind == cdk_array)
+	error_at (loc, "condition declares an array");
+      else
+	error_at (loc, "condition declares a function");
+      if (parser->fully_implicit_function_template_p)
+	abort_fully_implicit_template (parser);
+      cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true,
+					     /*or_comma=*/false,
+					     /*consume_paren=*/false);
+      return false;
+    }
+  else
+    return true;
+}
+
 /* Parse a condition.
 
    condition:
@@ -11571,6 +11599,7 @@ cp_parser_condition (cp_parser* parser)
       tree attributes;
       cp_declarator *declarator;
       tree initializer = NULL_TREE;
+      location_t loc = cp_lexer_peek_token (parser->lexer)->location;
 
       /* Parse the declarator.  */
       declarator = cp_parser_declarator (parser, CP_PARSER_DECLARATOR_NAMED,
@@ -11582,6 +11611,11 @@ cp_parser_condition (cp_parser* parser)
       attributes = cp_parser_attributes_opt (parser);
       /* Parse the asm-specification.  */
       asm_specification = cp_parser_asm_specification_opt (parser);
+
+      if (!cp_parser_error_occurred (parser)
+	  && !cp_parser_check_condition_declarator (parser, declarator, loc))
+	return error_mark_node;
+
       /* If the next token is not an `=' or '{', then we might still be
 	 looking at an expression.  For example:
 
Index: testsuite/g++.dg/cpp0x/cond1.C
===================================================================
--- testsuite/g++.dg/cpp0x/cond1.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/cond1.C	(working copy)
@@ -0,0 +1,23 @@
+// PR c++/84588
+// { dg-do compile { target c++11 } }
+
+void foo()
+{
+  if (int bar() {})  // { dg-error "condition declares a function" }
+    ;
+
+  for (;int bar() {};)  // { dg-error "condition declares a function" }
+    ;
+
+  while (int bar() {})  // { dg-error "condition declares a function" }
+    ;
+
+  if (int a[] {})  // { dg-error "condition declares an array" }
+    ;
+
+  for (;int a[] {};)  // { dg-error "condition declares an array" }
+    ;
+
+  while (int a[] {})  // { dg-error "condition declares an array" }
+    ;
+}
Index: testsuite/g++.dg/cpp1y/pr84588-1.C
===================================================================
--- testsuite/g++.dg/cpp1y/pr84588-1.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/pr84588-1.C	(working copy)
@@ -0,0 +1,25 @@
+// { dg-do compile { target c++14 } }
+
+struct a {
+  void b() {}
+  void c(void (*) () = [] {
+      if (a a(int auto) {})  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
+
+struct d {
+  void e() {}
+  void f(void (*) () = [] {
+      for (;d d(int auto) {};)  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
+
+struct g {
+  void h() {}
+  void i(void (*) () = [] {
+      while (g g(int auto) {})  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
Index: testsuite/g++.dg/cpp1y/pr84588-2.C
===================================================================
--- testsuite/g++.dg/cpp1y/pr84588-2.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/pr84588-2.C	(working copy)
@@ -0,0 +1,25 @@
+// { dg-do compile { target c++14 } }
+
+struct a {
+  void b() {}
+  void c(void (*) () = [] {
+      if (a a(int auto))  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
+
+struct d {
+  void e() {}
+  void f(void (*) () = [] {
+      for (;d d(int auto);)  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
+
+struct g {
+  void h() {}
+  void i(void (*) () = [] {
+      while (g g(int auto))  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
Index: testsuite/g++.dg/parse/cond6.C
===================================================================
--- testsuite/g++.dg/parse/cond6.C	(nonexistent)
+++ testsuite/g++.dg/parse/cond6.C	(working copy)
@@ -0,0 +1,22 @@
+// PR c++/84588
+
+void foo()
+{
+  if (int bar())  // { dg-error "condition declares a function" }
+    ;
+
+  for (;int bar();)  // { dg-error "condition declares a function" }
+    ;
+
+  while (int bar())  // { dg-error "condition declares a function" }
+    ;
+
+  if (int a[])  // { dg-error "condition declares an array" }
+    ;
+
+  for (;int a[];)  // { dg-error "condition declares an array" }
+    ;
+
+  while (int a[])  // { dg-error "condition declares an array" }
+    ;
+}
Index: testsuite/g++.old-deja/g++.jason/cond.C
===================================================================
--- testsuite/g++.old-deja/g++.jason/cond.C	(revision 260347)
+++ testsuite/g++.old-deja/g++.jason/cond.C	(working copy)
@@ -47,11 +47,10 @@ int main()
   if (struct B * foo = new B)
     ;
 
-  if (int f () = 1)		// { dg-warning "extern" "extern" } 
-  // { dg-error "is initialized like a variable" "var" { target *-*-* } .-1 }
+  if (int f () = 1)		// { dg-error "declares a function" } 
     ;
   
-  if (int a[2] = {1, 2})	// { dg-error "extended init" "" { target { ! c++11 } } }
+  if (int a[2] = {1, 2})	// { dg-error "declares an array" }
     ;
 
 }

Re: [C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[Jason Merrill]

On Fri, May 18, 2018 at 4:41 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,
>
>>> On 18/05/2018 01:21, Jason Merrill wrote:
>>>>
>>>> On Thu, May 17, 2018 at 5:54 PM, Paolo Carlini
>>>> <paolo.carlini@oracle.com> wrote:
>>>>>
>>>>> On 17/05/2018 16:58, Jason Merrill wrote:
>>>>>>
>>>>>> On Thu, May 17, 2018 at 10:27 AM, Paolo Carlini
>>>>>> <paolo.carlini@oracle.com> wrote:
>>>>>>>
>>>>>>> PS: maybe better using function_declarator_p?
>>>>>>
>>>>>> I think so, yes.  The relevant rule seems to be "The declarator shall
>>>>>> not specify a function or an array.", so let's check for arrays, too.
>>>>>
>>>>> Agreed. I had the amended patch ready when I noticed (again) that it
>>>>> wasn't
>>>>> addressing another related class of issues which involves declarators
>>>>> not
>>>>> followed by initializers. Thus I tried to fix those too, and the below
>>>>> which
>>>>> moves the check up appears to work fine, passes testing, etc. Are there
>>>>> any
>>>>> risks that an erroneous function / array as declarator is in fact a
>>>>> well
>>>>> formed expression?!?
>>>>
>>>> That doesn't matter; if it parses as a declarator, it's a declarator,
>>>> even if it's an ill-formed declarator.  But...
>>>>
>>>> +      bool decl_p = cp_parser_parse_definitely (parser);
>>>> +      if (!cp_parser_check_condition_declarator (parser, declarator,
>>>> loc))
>>>> +        return error_mark_node;
>>>>
>>>> ...if cp_parser_parse_definitely returns false, parsing as a
>>>> declarator failed, so we shouldn't look at "declarator".
>
> What I'm attaching below isn't affected by this problem: I moved the check
> further up - *before* maybe calling cp_parser_simulated_error because an
> initializer isn't in sight - and is carried out only when
> !cp_parser_error_occurred, thus cp_parser_declarator succeeded .
> cp_parser_commit_to_tentative_parse is called when the check fails.
> Bootstrapped and tested on x86_64-linux.

I had in mind moving the call to cp_parser_check_condition_declarator
into the block controlled by cp_parser_parse_definitely; this is a
semantic check that should follow the syntactic checks.  If there's no
initializer, it doesn't parse as a condition declaration, so we don't
want to complain about it being a semantically invalid condition
declaration.

Jason

Re: [C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[Paolo Carlini]

Hi,

On 18/05/2018 15:56, Jason Merrill wrote:
> I had in mind moving the call to cp_parser_check_condition_declarator
> into the block controlled by cp_parser_parse_definitely; this is a
> semantic check that should follow the syntactic checks.  If there's no
> initializer, it doesn't parse as a condition declaration, so we don't
> want to complain about it being a semantically invalid condition
> declaration.
If we do that we are back to something very, very, similar to what I 
posted at the beginning of the thread, right? Therefore, for all the 
testcases which don't have an initializer we end-up with *horrible*, 
literally *horrible* cascades of errors, plus we ICE on the c++/84588 
variants without an initializer. If you like we can do that and defer 
the other related issues - strictly speaking c++/84588 would be fixed - 
to another new bug.

Paolo.

Re: [C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[Jason Merrill]

On Fri, May 18, 2018 at 10:05 AM, Paolo Carlini
<paolo.carlini@oracle.com> wrote:
> Hi,
>
> On 18/05/2018 15:56, Jason Merrill wrote:
>>
>> I had in mind moving the call to cp_parser_check_condition_declarator
>> into the block controlled by cp_parser_parse_definitely; this is a
>> semantic check that should follow the syntactic checks.  If there's no
>> initializer, it doesn't parse as a condition declaration, so we don't
>> want to complain about it being a semantically invalid condition
>> declaration.
>
> If we do that we are back to something very, very, similar to what I posted
> at the beginning of the thread, right? Therefore, for all the testcases
> which don't have an initializer we end-up with *horrible*, literally
> *horrible* cascades of errors, plus we ICE on the c++/84588 variants without
> an initializer.

Ah.  Why is that?

I guess it's desirable to also give this error when the declarator is
followed by ) or ; rather than other tokens that could be more
expression (like in A(a).x in the comment).

Jason

Re: [C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[Paolo Carlini]

Hi,

On 18/05/2018 16:19, Jason Merrill wrote:
> On Fri, May 18, 2018 at 10:05 AM, Paolo Carlini
> <paolo.carlini@oracle.com> wrote:
>> Hi,
>>
>> On 18/05/2018 15:56, Jason Merrill wrote:
>>> I had in mind moving the call to cp_parser_check_condition_declarator
>>> into the block controlled by cp_parser_parse_definitely; this is a
>>> semantic check that should follow the syntactic checks.  If there's no
>>> initializer, it doesn't parse as a condition declaration, so we don't
>>> want to complain about it being a semantically invalid condition
>>> declaration.
>> If we do that we are back to something very, very, similar to what I posted
>> at the beginning of the thread, right? Therefore, for all the testcases
>> which don't have an initializer we end-up with *horrible*, literally
>> *horrible* cascades of errors, plus we ICE on the c++/84588 variants without
>> an initializer.
> Ah.  Why is that?
>
> I guess it's desirable to also give this error when the declarator is
> followed by ) or ; rather than other tokens that could be more
> expression (like in A(a).x in the comment).
I can certainly try to implement this, maybe just something minimal to 
begin with, as you say ) or ; alone would be safe and would already take 
care of all the tests I have around.

Certainly, unconditionally deferring at that point to 
cp_parser_expression leads to *very* bad error-recovery. For fun, try 
with 8.1.0:

     void foo()
     {
       for (;void bar(););
     }

and it gets worse.

Paolo.

Re: [C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[Jason Merrill]

On Fri, May 18, 2018 at 10:30 AM, Paolo Carlini
<paolo.carlini@oracle.com> wrote:
> Hi,
>
> On 18/05/2018 16:19, Jason Merrill wrote:
>>
>> On Fri, May 18, 2018 at 10:05 AM, Paolo Carlini
>> <paolo.carlini@oracle.com> wrote:
>>>
>>> Hi,
>>>
>>> On 18/05/2018 15:56, Jason Merrill wrote:
>>>>
>>>> I had in mind moving the call to cp_parser_check_condition_declarator
>>>> into the block controlled by cp_parser_parse_definitely; this is a
>>>> semantic check that should follow the syntactic checks.  If there's no
>>>> initializer, it doesn't parse as a condition declaration, so we don't
>>>> want to complain about it being a semantically invalid condition
>>>> declaration.
>>>
>>> If we do that we are back to something very, very, similar to what I
>>> posted
>>> at the beginning of the thread, right? Therefore, for all the testcases
>>> which don't have an initializer we end-up with *horrible*, literally
>>> *horrible* cascades of errors, plus we ICE on the c++/84588 variants
>>> without
>>> an initializer.
>>
>> Ah.  Why is that?
>>
>> I guess it's desirable to also give this error when the declarator is
>> followed by ) or ; rather than other tokens that could be more
>> expression (like in A(a).x in the comment).
>
> I can certainly try to implement this, maybe just something minimal to begin
> with, as you say ) or ; alone would be safe and would already take care of
> all the tests I have around.
>
> Certainly, unconditionally deferring at that point to cp_parser_expression
> leads to *very* bad error-recovery. For fun, try with 8.1.0:
>
>     void foo()
>     {
>       for (;void bar(););
>     }
>
> and it gets worse.

It would also need to be only for a non-parenthesized declarator,
which can't be an expression; a parenthesized declarator can be, as in
this well-formed testcase:

bool (bar()) { return 0; } // declaration

void foo()
{
  for (;bool (bar());); // expression
}

Jason

Re: [C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[Paolo Carlini]

Hi,

On 18/05/2018 16:45, Jason Merrill wrote:
>>> I guess it's desirable to also give this error when the declarator is
>>> followed by ) or ; rather than other tokens that could be more
>>> expression (like in A(a).x in the comment).
>> I can certainly try to implement this, maybe just something minimal to begin
>> with, as you say ) or ; alone would be safe and would already take care of
>> all the tests I have around.
>>
>> Certainly, unconditionally deferring at that point to cp_parser_expression
>> leads to *very* bad error-recovery. For fun, try with 8.1.0:
>>
>>      void foo()
>>      {
>>        for (;void bar(););
>>      }
>>
>> and it gets worse.
> It would also need to be only for a non-parenthesized declarator,
> which can't be an expression; a parenthesized declarator can be, as in
> this well-formed testcase:
>
> bool (bar()) { return 0; } // declaration
>
> void foo()
> {
>    for (;bool (bar());); // expression
> }
Thanks for clarifying that. I'll make sure we have such testcases.

Interestingly, if we aren't careful about that in the new code, 
libstdc++-v3 doesn't even build, eh!

Paolo.

Re: [C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[Paolo Carlini]

Hi again,

I'm playing with a wild, wild idea: would it make sense to try *first* 
an expression? Because, a quickly hacked draft appears to handle very 
elegantly all the possible cases of "junk" after the declarator, eg:

     void foo()
     {
         if (void bar()JUNK);
      }

and the parenthesized case, etc, etc. Before trying to seriously work on 
that I wanted to ask...

Paolo.

Re: [C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[Jason Merrill]

On Fri, May 18, 2018 at 1:40 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi again,
>
> I'm playing with a wild, wild idea: would it make sense to try *first* an
> expression? Because, a quickly hacked draft appears to handle very elegantly
> all the possible cases of "junk" after the declarator, eg:
>
>     void foo()
>     {
>         if (void bar()JUNK);
>      }
>
> and the parenthesized case, etc, etc. Before trying to seriously work on
> that I wanted to ask...

We'd need to try parsing as a declaration whether or not parsing as an
expression works, since any ambiguous cases are treated as
declarations [stmt.ambig].

Jason

Re: [C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 2228 bytes --]

Hi,

On 19/05/2018 01:40, Jason Merrill wrote:
> On Fri, May 18, 2018 at 1:40 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
>> Hi again,
>>
>> I'm playing with a wild, wild idea: would it make sense to try *first* an
>> expression? Because, a quickly hacked draft appears to handle very elegantly
>> all the possible cases of "junk" after the declarator, eg:
>>
>>      void foo()
>>      {
>>          if (void bar()JUNK);
>>       }
>>
>> and the parenthesized case, etc, etc. Before trying to seriously work on
>> that I wanted to ask...
> We'd need to try parsing as a declaration whether or not parsing as an
> expression works, since any ambiguous cases are treated as
> declarations [stmt.ambig].
Yeah, that complicates the implementation of my idea. However, I'm 
thinking that at least *in the specific case of cp_parse_condition* from 
the computational complexity point of view probably we wouldn't regress 
much, because declarations are rare anyway, thus in most of the cases we 
end up doing both anyway. If we do expressions first and we save the 
result, then I believe when we can handle the declarator as something 
which cannot be a function or an array even if we don't see the 
initializer much more easily, we easily have a better diagnostic for 
things like

     if (int x);

(another case we currently handle pretty badly, we don't talk about the 
missing initializer at all!), we cope elegantly with any junk following 
the wrong function/array declaration, etc. See that attached, if you are 
curious, which essentially passes the testsuite modulo a nit (*) which 
doesn't have anything to do with [stmt.ambig] per se (which of course is 
*not* correctly implemented in the wip).

Can you give me your opinion about the more detailed idea, in particular 
whether we already have good infrastructure to implement [stmt.ambig] in 
this context, thus to keep the first parsing as an expression around, 
possibly returning to it if the parsing as a declaration fails??

I hope I'm making sense, it's again late here... in any case the wip 
patch I did much earlier today ;)

Paolo.

(*) Has to do with David's access failure hint not handling deferred 
access checks (accessor-fixits-6.C).

[-- Attachment #2: p_84588_in_prog --]
[-- Type: text/plain, Size: 4823 bytes --]

Index: parser.c
===================================================================
--- parser.c	(revision 260347)
+++ parser.c	(working copy)
@@ -11527,6 +11527,33 @@ cp_parser_selection_statement (cp_parser* parser,
     }
 }
 
+/* Helper function for cp_parser_condition.  Enforces [stmt.stmt]/2:
+   The declarator shall not specify a function or an array.  Returns
+   TRUE if the declarator is valid, FALSE otherwise.  */
+
+static bool
+cp_parser_check_condition_declarator (cp_parser* parser,
+				      cp_declarator *declarator,
+				      location_t loc)
+{
+  if (function_declarator_p (declarator)
+      || declarator->kind == cdk_array)
+    {
+      if (declarator->kind == cdk_array)
+	error_at (loc, "condition declares an array");
+      else
+	error_at (loc, "condition declares a function");
+      if (parser->fully_implicit_function_template_p)
+	abort_fully_implicit_template (parser);
+      cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true,
+					     /*or_comma=*/false,
+      					     /*consume_paren=*/false);
+      return false;
+    }
+  else
+    return true;
+}
+
 /* Parse a condition.
 
    condition:
@@ -11545,12 +11572,20 @@ cp_parser_selection_statement (cp_parser* parser,
 static tree
 cp_parser_condition (cp_parser* parser)
 {
+  cp_parser_parse_tentatively (parser);
+
+  /* Try the expression first.  */
+  tree expr = cp_parser_expression (parser);
+
+  if (cp_parser_parse_definitely (parser))
+    return expr;
+
+  cp_parser_parse_tentatively (parser);
+
   cp_decl_specifier_seq type_specifiers;
   const char *saved_message;
   int declares_class_or_enum;
 
-  /* Try the declaration first.  */
-  cp_parser_parse_tentatively (parser);
   /* New types are not allowed in the type-specifier-seq for a
      condition.  */
   saved_message = parser->type_definition_forbidden_message;
@@ -11563,6 +11598,7 @@ cp_parser_condition (cp_parser* parser)
 				&declares_class_or_enum);
   /* Restore the saved message.  */
   parser->type_definition_forbidden_message = saved_message;
+
   /* If all is well, we might be looking at a declaration.  */
   if (!cp_parser_error_occurred (parser))
     {
@@ -11570,7 +11606,8 @@ cp_parser_condition (cp_parser* parser)
       tree asm_specification;
       tree attributes;
       cp_declarator *declarator;
-      tree initializer = NULL_TREE;
+      tree initializer;
+      location_t loc = cp_lexer_peek_token (parser->lexer)->location;
 
       /* Parse the declarator.  */
       declarator = cp_parser_declarator (parser, CP_PARSER_DECLARATOR_NAMED,
@@ -11582,19 +11619,7 @@ cp_parser_condition (cp_parser* parser)
       attributes = cp_parser_attributes_opt (parser);
       /* Parse the asm-specification.  */
       asm_specification = cp_parser_asm_specification_opt (parser);
-      /* If the next token is not an `=' or '{', then we might still be
-	 looking at an expression.  For example:
 
-	   if (A(a).x)
-
-	 looks like a decl-specifier-seq and a declarator -- but then
-	 there is no `=', so this is an expression.  */
-      if (cp_lexer_next_token_is_not (parser->lexer, CPP_EQ)
-	  && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_BRACE))
-	cp_parser_simulate_error (parser);
-	
-      /* If we did see an `=' or '{', then we are looking at a declaration
-	 for sure.  */
       if (cp_parser_parse_definitely (parser))
 	{
 	  tree pushed_scope;
@@ -11601,6 +11626,9 @@ cp_parser_condition (cp_parser* parser)
 	  bool non_constant_p;
 	  int flags = LOOKUP_ONLYCONVERTING;
 
+	  if (!cp_parser_check_condition_declarator (parser, declarator, loc))
+	    return error_mark_node;
+
 	  /* Create the declaration.  */
 	  decl = start_decl (declarator, &type_specifiers,
 			     /*initialized_p=*/true,
@@ -11614,12 +11642,18 @@ cp_parser_condition (cp_parser* parser)
 	      CONSTRUCTOR_IS_DIRECT_INIT (initializer) = 1;
 	      flags = 0;
 	    }
-	  else
+	  else if (cp_lexer_next_token_is (parser->lexer, CPP_EQ))
 	    {
 	      /* Consume the `='.  */
 	      cp_parser_require (parser, CPP_EQ, RT_EQ);
-	      initializer = cp_parser_initializer_clause (parser, &non_constant_p);
+	      initializer = cp_parser_initializer_clause (parser,
+							  &non_constant_p);
 	    }
+	  else
+	    {
+	      cp_parser_error (parser, "expected initializer");
+	      initializer = error_mark_node;
+	    }
 	  if (BRACE_ENCLOSED_INITIALIZER_P (initializer))
 	    maybe_warn_cpp0x (CPP0X_INITIALIZER_LISTS);
 
@@ -11640,8 +11674,8 @@ cp_parser_condition (cp_parser* parser)
   else
     cp_parser_abort_tentative_parse (parser);
 
-  /* Otherwise, we are looking at an expression.  */
-  return cp_parser_expression (parser);
+  cp_parser_error (parser, "expected condition");
+  return error_mark_node;
 }
 
 /* Parses a for-statement or range-for-statement until the closing ')',

Re: [C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[Jason Merrill]

On Fri, May 18, 2018 at 8:27 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> On 19/05/2018 01:40, Jason Merrill wrote:
>> On Fri, May 18, 2018 at 1:40 PM, Paolo Carlini <paolo.carlini@oracle.com>
>> wrote:
>>>
>>> Hi again,
>>>
>>> I'm playing with a wild, wild idea: would it make sense to try *first* an
>>> expression? Because, a quickly hacked draft appears to handle very
>>> elegantly
>>> all the possible cases of "junk" after the declarator, eg:
>>>
>>>      void foo()
>>>      {
>>>          if (void bar()JUNK);
>>>       }
>>>
>>> and the parenthesized case, etc, etc. Before trying to seriously work on
>>> that I wanted to ask...
>>
>> We'd need to try parsing as a declaration whether or not parsing as an
>> expression works, since any ambiguous cases are treated as
>> declarations [stmt.ambig].
>
> Yeah, that complicates the implementation of my idea. However, I'm thinking
> that at least *in the specific case of cp_parse_condition* from the
> computational complexity point of view probably we wouldn't regress much,
> because declarations are rare anyway, thus in most of the cases we end up
> doing both anyway. If we do expressions first and we save the result, then I
> believe when we can handle the declarator as something which cannot be a
> function or an array even if we don't see the initializer much more easily,
> we easily have a better diagnostic for things like
>
>     if (int x);
>
> (another case we currently handle pretty badly, we don't talk about the
> missing initializer at all!), we cope elegantly with any junk following the
> wrong function/array declaration, etc. See that attached, if you are
> curious, which essentially passes the testsuite modulo a nit (*) which
> doesn't have anything to do with [stmt.ambig] per se (which of course is
> *not* correctly implemented in the wip).
>
> Can you give me your opinion about the more detailed idea, in particular
> whether we already have good infrastructure to implement [stmt.ambig] in
> this context, thus to keep the first parsing as an expression around,
> possibly returning to it if the parsing as a declaration fails??

I would expect it to cause different diagnostic issues, from
complaining about something not being a proper declaration when it's
really an expression.  I also wonder about warning problems (either
missed or bogus) due to trying these in a different order.

How about doing cp_parser_commit_to_tentative_parse if we see
something that must be a declaration?  cp_parser_simple_declaration
has

  /* If we have seen at least one decl-specifier, and the next token
     is not a parenthesis, then we must be looking at a declaration.
     (After "int (" we might be looking at a functional cast.)  */
  if (decl_specifiers.any_specifiers_p
      && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_PAREN)
      && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_BRACE)
      && !cp_parser_error_occurred (parser))
    cp_parser_commit_to_tentative_parse (parser);

That seems useful here, as well.  Maybe factored into a separate function.

Jason

Re: [C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 1977 bytes --]

Hi,

On 19/05/2018 15:30, Jason Merrill wrote:
> I would expect it to cause different diagnostic issues, from
> complaining about something not being a proper declaration when it's
> really an expression.  I also wonder about warning problems (either
> missed or bogus) due to trying these in a different order.
Yes. It seems kind of science-fiction project ;) I'll give it more 
thought, anyway...
> How about doing cp_parser_commit_to_tentative_parse if we see
> something that must be a declaration?  cp_parser_simple_declaration
> has
>
>    /* If we have seen at least one decl-specifier, and the next token
>       is not a parenthesis, then we must be looking at a declaration.
>       (After "int (" we might be looking at a functional cast.)  */
>    if (decl_specifiers.any_specifiers_p
>        && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_PAREN)
>        && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_BRACE)
>        && !cp_parser_error_occurred (parser))
>      cp_parser_commit_to_tentative_parse (parser);
>
> That seems useful here, as well.  Maybe factored into a separate function.
Hmm, I see, thanks for the tip. To other day, eventually I figured out 
the below and fully tested it, which seems quite good to me, frankly: it 
simply adds a check of parenthesized_p (per your highly welcome previous 
clarification) to the existing checks: having spent quite a bit of time 
on [dcl.ambig.res] I think it's Ok - if the declarator isn't 
parenthesized can't be in fact an expression - and alone that allows us 
to make very good progress - well beyond the requirements of c++/84588 - 
on a number of diagnostic-quality fronts, see the attached additional 
testcases too. I can still construct a nasty condition of the form, say, 
'a (a(int auto)JUNK' which we don't handle well in terms of error 
recovery, but if you agree that the below is correct, it's probably 
enough for the *regression* part...

Thanks!
Paolo.

/////////////////////

[-- Attachment #2: patch_84588_10 --]
[-- Type: text/plain, Size: 9862 bytes --]

Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 260402)
+++ cp/parser.c	(working copy)
@@ -11527,6 +11527,33 @@ cp_parser_selection_statement (cp_parser* parser,
     }
 }
 
+/* Helper function for cp_parser_condition.  Enforces [stmt.stmt]/2:
+   The declarator shall not specify a function or an array.  Returns
+   TRUE if the declarator is valid, FALSE otherwise.  */
+
+static bool
+cp_parser_check_condition_declarator (cp_parser* parser,
+                                     cp_declarator *declarator,
+                                     location_t loc)
+{
+  if (function_declarator_p (declarator)
+      || declarator->kind == cdk_array)
+    {
+      if (declarator->kind == cdk_array)
+       error_at (loc, "condition declares an array");
+      else
+       error_at (loc, "condition declares a function");
+      if (parser->fully_implicit_function_template_p)
+       abort_fully_implicit_template (parser);
+      cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true,
+                                            /*or_comma=*/false,
+                                            /*consume_paren=*/false);
+      return false;
+    }
+  else
+    return true;
+}
+
 /* Parse a condition.
 
    condition:
@@ -11571,11 +11598,13 @@ cp_parser_condition (cp_parser* parser)
       tree attributes;
       cp_declarator *declarator;
       tree initializer = NULL_TREE;
+      bool parenthesized_p = false;
+      location_t loc = cp_lexer_peek_token (parser->lexer)->location;
 
       /* Parse the declarator.  */
       declarator = cp_parser_declarator (parser, CP_PARSER_DECLARATOR_NAMED,
 					 /*ctor_dtor_or_conv_p=*/NULL,
-					 /*parenthesized_p=*/NULL,
+					 &parenthesized_p,
 					 /*member_p=*/false,
 					 /*friend_p=*/false);
       /* Parse the attributes.  */
@@ -11582,8 +11611,9 @@ cp_parser_condition (cp_parser* parser)
       attributes = cp_parser_attributes_opt (parser);
       /* Parse the asm-specification.  */
       asm_specification = cp_parser_asm_specification_opt (parser);
-      /* If the next token is not an `=' or '{', then we might still be
-	 looking at an expression.  For example:
+      /* If the next token is not an `=' or '{' and the declarator is
+	 parenthesized, then we might still be looking at an expression.
+	 For example:
 
 	   if (A(a).x)
 
@@ -11590,11 +11620,12 @@ cp_parser_condition (cp_parser* parser)
 	 looks like a decl-specifier-seq and a declarator -- but then
 	 there is no `=', so this is an expression.  */
       if (cp_lexer_next_token_is_not (parser->lexer, CPP_EQ)
-	  && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_BRACE))
+	  && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_BRACE)
+	  && parenthesized_p)
 	cp_parser_simulate_error (parser);
 	
-      /* If we did see an `=' or '{', then we are looking at a declaration
-	 for sure.  */
+      /* If we did see an `=' or '{' or the declarator isn't parenthesized,
+	 then we are looking at a declaration for sure.  */
       if (cp_parser_parse_definitely (parser))
 	{
 	  tree pushed_scope;
@@ -11601,6 +11632,9 @@ cp_parser_condition (cp_parser* parser)
 	  bool non_constant_p;
 	  int flags = LOOKUP_ONLYCONVERTING;
 
+	  if (!cp_parser_check_condition_declarator (parser, declarator, loc))
+	    return error_mark_node;
+
 	  /* Create the declaration.  */
 	  decl = start_decl (declarator, &type_specifiers,
 			     /*initialized_p=*/true,
@@ -11614,12 +11648,18 @@ cp_parser_condition (cp_parser* parser)
 	      CONSTRUCTOR_IS_DIRECT_INIT (initializer) = 1;
 	      flags = 0;
 	    }
-	  else
+	  else if (cp_lexer_next_token_is (parser->lexer, CPP_EQ))
 	    {
 	      /* Consume the `='.  */
-	      cp_parser_require (parser, CPP_EQ, RT_EQ);
-	      initializer = cp_parser_initializer_clause (parser, &non_constant_p);
+	      cp_lexer_consume_token (parser->lexer);
+	      initializer = cp_parser_initializer_clause (parser,
+							  &non_constant_p);
 	    }
+	  else
+	    {
+	      cp_parser_error (parser, "expected initializer");
+	      initializer = error_mark_node;
+	    }
 	  if (BRACE_ENCLOSED_INITIALIZER_P (initializer))
 	    maybe_warn_cpp0x (CPP0X_INITIALIZER_LISTS);
 
Index: testsuite/g++.dg/cpp0x/cond1.C
===================================================================
--- testsuite/g++.dg/cpp0x/cond1.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/cond1.C	(working copy)
@@ -0,0 +1,17 @@
+// PR c++/84588
+// { dg-do compile { target c++11 } }
+
+void foo()
+{
+  if (int bar() {});  // { dg-error "condition declares a function" }
+
+  for (;int bar() {};);  // { dg-error "condition declares a function" }
+
+  while (int bar() {});  // { dg-error "condition declares a function" }
+
+  if (int a[] {});  // { dg-error "condition declares an array" }
+
+  for (;int a[] {};);  // { dg-error "condition declares an array" }
+
+  while (int a[] {});  // { dg-error "condition declares an array" }
+}
Index: testsuite/g++.dg/cpp1y/pr84588-1.C
===================================================================
--- testsuite/g++.dg/cpp1y/pr84588-1.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/pr84588-1.C	(working copy)
@@ -0,0 +1,25 @@
+// { dg-do compile { target c++14 } }
+
+struct a {
+  void b() {}
+  void c(void (*) () = [] {
+      if (a a(int auto) {})  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
+
+struct d {
+  void e() {}
+  void f(void (*) () = [] {
+      for (;d d(int auto) {};)  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
+
+struct g {
+  void h() {}
+  void i(void (*) () = [] {
+      while (g g(int auto) {})  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
Index: testsuite/g++.dg/cpp1y/pr84588-2.C
===================================================================
--- testsuite/g++.dg/cpp1y/pr84588-2.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/pr84588-2.C	(working copy)
@@ -0,0 +1,25 @@
+// { dg-do compile { target c++14 } }
+
+struct a {
+  void b() {}
+  void c(void (*) () = [] {
+      if (a a(int auto))  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
+
+struct d {
+  void e() {}
+  void f(void (*) () = [] {
+      for (;d d(int auto);)  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
+
+struct g {
+  void h() {}
+  void i(void (*) () = [] {
+      while (g g(int auto))  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
Index: testsuite/g++.dg/cpp1y/pr84588-3.C
===================================================================
--- testsuite/g++.dg/cpp1y/pr84588-3.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/pr84588-3.C	(working copy)
@@ -0,0 +1,25 @@
+// { dg-do compile { target c++14 } }
+
+struct a {
+  void b() {}
+  void c(void (*) () = [] {
+      if (a a(int auto)JUNK)  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
+
+struct d {
+  void e() {}
+  void f(void (*) () = [] {
+      for (;d d(int auto)JUNK;)  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
+
+struct g {
+  void h() {}
+  void i(void (*) () = [] {
+      while (g g(int auto)JUNK)  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
Index: testsuite/g++.dg/parse/cond6.C
===================================================================
--- testsuite/g++.dg/parse/cond6.C	(nonexistent)
+++ testsuite/g++.dg/parse/cond6.C	(working copy)
@@ -0,0 +1,16 @@
+// PR c++/84588
+
+void foo()
+{
+  if (int bar());  // { dg-error "condition declares a function" }
+
+  for (;int bar(););  // { dg-error "condition declares a function" }
+
+  while (int bar());  // { dg-error "condition declares a function" }
+
+  if (int a[]);  // { dg-error "condition declares an array" }
+
+  for (;int a[];);  // { dg-error "condition declares an array" }
+
+  while (int a[]);  // { dg-error "condition declares an array" }
+}
Index: testsuite/g++.dg/parse/cond7.C
===================================================================
--- testsuite/g++.dg/parse/cond7.C	(nonexistent)
+++ testsuite/g++.dg/parse/cond7.C	(working copy)
@@ -0,0 +1,18 @@
+// PR c++/84588
+
+bool (bar()) { return 0; } // declaration
+
+void foo1()
+{
+  if (bool (bar())); // expression
+}
+
+void foo2()
+{
+  for (;bool (bar());); // expression
+}
+
+void foo3()
+{
+  while (bool (bar())); // expression
+}
Index: testsuite/g++.dg/parse/cond8.C
===================================================================
--- testsuite/g++.dg/parse/cond8.C	(nonexistent)
+++ testsuite/g++.dg/parse/cond8.C	(working copy)
@@ -0,0 +1,16 @@
+// PR c++/84588
+
+void foo()
+{
+  if (int x);  // { dg-error "expected initializer" }
+
+  for (;int x;);  // { dg-error "expected initializer" }
+
+  while (int x);  // { dg-error "expected initializer" }
+
+  if (int x);  // { dg-error "expected initializer" }
+
+  for (;int x;);  // { dg-error "expected initializer" }
+
+  while (int x);  // { dg-error "expected initializer" }
+}
Index: testsuite/g++.old-deja/g++.jason/cond.C
===================================================================
--- testsuite/g++.old-deja/g++.jason/cond.C	(revision 260402)
+++ testsuite/g++.old-deja/g++.jason/cond.C	(working copy)
@@ -47,11 +47,10 @@ int main()
   if (struct B * foo = new B)
     ;
 
-  if (int f () = 1)		// { dg-warning "extern" "extern" } 
-  // { dg-error "is initialized like a variable" "var" { target *-*-* } .-1 }
+  if (int f () = 1)		// { dg-error "declares a function" } 
     ;
   
-  if (int a[2] = {1, 2})	// { dg-error "extended init" "" { target { ! c++11 } } }
+  if (int a[2] = {1, 2})	// { dg-error "declares an array" }
     ;
 
 }

Re: [C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 1111 bytes --]

Hi again,

On 19/05/2018 15:30, Jason Merrill wrote:
> How about doing cp_parser_commit_to_tentative_parse if we see
> something that must be a declaration?  cp_parser_simple_declaration
> has
>
>    /* If we have seen at least one decl-specifier, and the next token
>       is not a parenthesis, then we must be looking at a declaration.
>       (After "int (" we might be looking at a functional cast.)  */
>    if (decl_specifiers.any_specifiers_p
>        && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_PAREN)
>        && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_BRACE)
>        && !cp_parser_error_occurred (parser))
>      cp_parser_commit_to_tentative_parse (parser);
>
> That seems useful here, as well.  Maybe factored into a separate function.
The below implements this new idea, which indeed appears to work well: I 
tested it and testsuite-wise seems essentially equivalent to what I 
posted yesterday, besides a slightly worse error-recovery for the first 
issue in cpp1z/decomp16.C: an additional 'no match for ‘operator=’' error.

Thanks!
Paolo.

//////////////////


[-- Attachment #2: patch_84588_11 --]
[-- Type: text/plain, Size: 10565 bytes --]

Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 260402)
+++ cp/parser.c	(working copy)
@@ -11527,6 +11527,53 @@ cp_parser_selection_statement (cp_parser* parser,
     }
 }
 
+/* Helper function for cp_parser_condition and cp_parser_simple_declaration.
+   If we have seen at least one decl-specifier, and the next token
+   is not a parenthesis, then we must be looking at a declaration.
+   (After "int (" we might be looking at a functional cast.)  */
+
+static bool
+cp_parser_maybe_commit_to_declaration (cp_parser* parser,
+				       bool any_specifiers_p)
+{
+  if (any_specifiers_p
+      && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_PAREN)
+      && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_BRACE)
+      && !cp_parser_error_occurred (parser))
+    {
+      cp_parser_commit_to_tentative_parse (parser);
+      return true;
+    }
+  return false;
+}
+
+/* Helper function for cp_parser_condition.  Enforces [stmt.stmt]/2:
+   The declarator shall not specify a function or an array.  Returns
+   TRUE if the declarator is valid, FALSE otherwise.  */
+
+static bool
+cp_parser_check_condition_declarator (cp_parser* parser,
+                                     cp_declarator *declarator,
+                                     location_t loc)
+{
+  if (function_declarator_p (declarator)
+      || declarator->kind == cdk_array)
+    {
+      if (declarator->kind == cdk_array)
+       error_at (loc, "condition declares an array");
+      else
+       error_at (loc, "condition declares a function");
+      if (parser->fully_implicit_function_template_p)
+       abort_fully_implicit_template (parser);
+      cp_parser_skip_to_closing_parenthesis (parser, /*recovering=*/true,
+                                            /*or_comma=*/false,
+                                            /*consume_paren=*/false);
+      return false;
+    }
+  else
+    return true;
+}
+
 /* Parse a condition.
 
    condition:
@@ -11563,6 +11610,10 @@ cp_parser_condition (cp_parser* parser)
 				&declares_class_or_enum);
   /* Restore the saved message.  */
   parser->type_definition_forbidden_message = saved_message;
+
+  cp_parser_maybe_commit_to_declaration (parser,
+					 type_specifiers.any_specifiers_p);
+
   /* If all is well, we might be looking at a declaration.  */
   if (!cp_parser_error_occurred (parser))
     {
@@ -11571,6 +11622,7 @@ cp_parser_condition (cp_parser* parser)
       tree attributes;
       cp_declarator *declarator;
       tree initializer = NULL_TREE;
+      location_t loc = cp_lexer_peek_token (parser->lexer)->location;
 
       /* Parse the declarator.  */
       declarator = cp_parser_declarator (parser, CP_PARSER_DECLARATOR_NAMED,
@@ -11601,6 +11653,9 @@ cp_parser_condition (cp_parser* parser)
 	  bool non_constant_p;
 	  int flags = LOOKUP_ONLYCONVERTING;
 
+	  if (!cp_parser_check_condition_declarator (parser, declarator, loc))
+	    return error_mark_node;
+
 	  /* Create the declaration.  */
 	  decl = start_decl (declarator, &type_specifiers,
 			     /*initialized_p=*/true,
@@ -11614,12 +11669,18 @@ cp_parser_condition (cp_parser* parser)
 	      CONSTRUCTOR_IS_DIRECT_INIT (initializer) = 1;
 	      flags = 0;
 	    }
-	  else
+	  else if (cp_lexer_next_token_is (parser->lexer, CPP_EQ))
 	    {
 	      /* Consume the `='.  */
-	      cp_parser_require (parser, CPP_EQ, RT_EQ);
-	      initializer = cp_parser_initializer_clause (parser, &non_constant_p);
+	      cp_lexer_consume_token (parser->lexer);
+	      initializer = cp_parser_initializer_clause (parser,
+							  &non_constant_p);
 	    }
+	  else
+	    {
+	      cp_parser_error (parser, "expected initializer");
+	      initializer = error_mark_node;
+	    }
 	  if (BRACE_ENCLOSED_INITIALIZER_P (initializer))
 	    maybe_warn_cpp0x (CPP0X_INITIALIZER_LISTS);
 
@@ -12936,14 +12997,8 @@ cp_parser_simple_declaration (cp_parser* parser,
       goto done;
     }
 
-  /* If we have seen at least one decl-specifier, and the next token
-     is not a parenthesis, then we must be looking at a declaration.
-     (After "int (" we might be looking at a functional cast.)  */
-  if (decl_specifiers.any_specifiers_p
-      && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_PAREN)
-      && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_BRACE)
-      && !cp_parser_error_occurred (parser))
-    cp_parser_commit_to_tentative_parse (parser);
+  cp_parser_maybe_commit_to_declaration (parser,
+					 decl_specifiers.any_specifiers_p);
 
   /* Look for C++17 decomposition declaration.  */
   for (size_t n = 1; ; n++)
Index: testsuite/g++.dg/cpp0x/cond1.C
===================================================================
--- testsuite/g++.dg/cpp0x/cond1.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/cond1.C	(working copy)
@@ -0,0 +1,17 @@
+// PR c++/84588
+// { dg-do compile { target c++11 } }
+
+void foo()
+{
+  if (int bar() {});  // { dg-error "condition declares a function" }
+
+  for (;int bar() {};);  // { dg-error "condition declares a function" }
+
+  while (int bar() {});  // { dg-error "condition declares a function" }
+
+  if (int a[] {});  // { dg-error "condition declares an array" }
+
+  for (;int a[] {};);  // { dg-error "condition declares an array" }
+
+  while (int a[] {});  // { dg-error "condition declares an array" }
+}
Index: testsuite/g++.dg/cpp1y/pr84588-1.C
===================================================================
--- testsuite/g++.dg/cpp1y/pr84588-1.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/pr84588-1.C	(working copy)
@@ -0,0 +1,25 @@
+// { dg-do compile { target c++14 } }
+
+struct a {
+  void b() {}
+  void c(void (*) () = [] {
+      if (a a(int auto) {})  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
+
+struct d {
+  void e() {}
+  void f(void (*) () = [] {
+      for (;d d(int auto) {};)  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
+
+struct g {
+  void h() {}
+  void i(void (*) () = [] {
+      while (g g(int auto) {})  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
Index: testsuite/g++.dg/cpp1y/pr84588-2.C
===================================================================
--- testsuite/g++.dg/cpp1y/pr84588-2.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/pr84588-2.C	(working copy)
@@ -0,0 +1,25 @@
+// { dg-do compile { target c++14 } }
+
+struct a {
+  void b() {}
+  void c(void (*) () = [] {
+      if (a a(int auto))  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
+
+struct d {
+  void e() {}
+  void f(void (*) () = [] {
+      for (;d d(int auto);)  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
+
+struct g {
+  void h() {}
+  void i(void (*) () = [] {
+      while (g g(int auto))  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
Index: testsuite/g++.dg/cpp1y/pr84588-3.C
===================================================================
--- testsuite/g++.dg/cpp1y/pr84588-3.C	(nonexistent)
+++ testsuite/g++.dg/cpp1y/pr84588-3.C	(working copy)
@@ -0,0 +1,25 @@
+// { dg-do compile { target c++14 } }
+
+struct a {
+  void b() {}
+  void c(void (*) () = [] {
+      if (a a(int auto)JUNK)  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
+
+struct d {
+  void e() {}
+  void f(void (*) () = [] {
+      for (;d d(int auto)JUNK;)  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
+
+struct g {
+  void h() {}
+  void i(void (*) () = [] {
+      while (g g(int auto)JUNK)  // { dg-error "two or more data types|condition declares a function" }
+      ;
+  }) {}
+};
Index: testsuite/g++.dg/cpp1z/decomp16.C
===================================================================
--- testsuite/g++.dg/cpp1z/decomp16.C	(revision 260402)
+++ testsuite/g++.dg/cpp1z/decomp16.C	(working copy)
@@ -8,7 +8,7 @@ void
 foo ()
 {
   auto [ a, b ] = A ();
-  for (; auto [ a, b ] = A (); )			// { dg-error "expected" }
+  for (; auto [ a, b ] = A (); )			// { dg-error "expected|no match" }
     ;
   for (; false; auto [ a, b ] = A ())			// { dg-error "expected" }
     ;
Index: testsuite/g++.dg/parse/cond6.C
===================================================================
--- testsuite/g++.dg/parse/cond6.C	(nonexistent)
+++ testsuite/g++.dg/parse/cond6.C	(working copy)
@@ -0,0 +1,16 @@
+// PR c++/84588
+
+void foo()
+{
+  if (int bar());  // { dg-error "condition declares a function" }
+
+  for (;int bar(););  // { dg-error "condition declares a function" }
+
+  while (int bar());  // { dg-error "condition declares a function" }
+
+  if (int a[]);  // { dg-error "condition declares an array" }
+
+  for (;int a[];);  // { dg-error "condition declares an array" }
+
+  while (int a[]);  // { dg-error "condition declares an array" }
+}
Index: testsuite/g++.dg/parse/cond7.C
===================================================================
--- testsuite/g++.dg/parse/cond7.C	(nonexistent)
+++ testsuite/g++.dg/parse/cond7.C	(working copy)
@@ -0,0 +1,18 @@
+// PR c++/84588
+
+bool (bar()) { return 0; } // declaration
+
+void foo1()
+{
+  if (bool (bar())); // expression
+}
+
+void foo2()
+{
+  for (;bool (bar());); // expression
+}
+
+void foo3()
+{
+  while (bool (bar())); // expression
+}
Index: testsuite/g++.dg/parse/cond8.C
===================================================================
--- testsuite/g++.dg/parse/cond8.C	(nonexistent)
+++ testsuite/g++.dg/parse/cond8.C	(working copy)
@@ -0,0 +1,10 @@
+// PR c++/84588
+
+void foo()
+{
+  if (int x);  // { dg-error "expected initializer" }
+
+  for (;int x;);  // { dg-error "expected initializer" }
+
+  while (int x);  // { dg-error "expected initializer" }
+}
Index: testsuite/g++.old-deja/g++.jason/cond.C
===================================================================
--- testsuite/g++.old-deja/g++.jason/cond.C	(revision 260402)
+++ testsuite/g++.old-deja/g++.jason/cond.C	(working copy)
@@ -47,11 +47,10 @@ int main()
   if (struct B * foo = new B)
     ;
 
-  if (int f () = 1)		// { dg-warning "extern" "extern" } 
-  // { dg-error "is initialized like a variable" "var" { target *-*-* } .-1 }
+  if (int f () = 1)		// { dg-error "declares a function" } 
     ;
   
-  if (int a[2] = {1, 2})	// { dg-error "extended init" "" { target { ! c++11 } } }
+  if (int a[2] = {1, 2})	// { dg-error "declares an array" }
     ;
 
 }

Re: [C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[Jason Merrill]

OK.

On Mon, May 21, 2018 at 8:41 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi again,
>
> On 19/05/2018 15:30, Jason Merrill wrote:
>>
>> How about doing cp_parser_commit_to_tentative_parse if we see
>> something that must be a declaration?  cp_parser_simple_declaration
>> has
>>
>>    /* If we have seen at least one decl-specifier, and the next token
>>       is not a parenthesis, then we must be looking at a declaration.
>>       (After "int (" we might be looking at a functional cast.)  */
>>    if (decl_specifiers.any_specifiers_p
>>        && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_PAREN)
>>        && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_BRACE)
>>        && !cp_parser_error_occurred (parser))
>>      cp_parser_commit_to_tentative_parse (parser);
>>
>> That seems useful here, as well.  Maybe factored into a separate function.
>
> The below implements this new idea, which indeed appears to work well: I
> tested it and testsuite-wise seems essentially equivalent to what I posted
> yesterday, besides a slightly worse error-recovery for the first issue in
> cpp1z/decomp16.C: an additional 'no match for ‘operator=’' error.
>
> Thanks!
> Paolo.
>
> //////////////////
>

Re: [C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())")​ (Take 2)

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 1440 bytes --]

On Mon, May 21, 2018 at 12:13 PM, Jason Merrill <jason@redhat.com> wrote:
> On Mon, May 21, 2018 at 8:41 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
>> On 19/05/2018 15:30, Jason Merrill wrote:

>>> How about doing cp_parser_commit_to_tentative_parse if we see
>>> something that must be a declaration?  cp_parser_simple_declaration
>>> has
>>>
>>>    /* If we have seen at least one decl-specifier, and the next token
>>>       is not a parenthesis, then we must be looking at a declaration.
>>>       (After "int (" we might be looking at a functional cast.)  */
>>>    if (decl_specifiers.any_specifiers_p
>>>        && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_PAREN)
>>>        && cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_BRACE)
>>>        && !cp_parser_error_occurred (parser))
>>>      cp_parser_commit_to_tentative_parse (parser);
>>>
>>> That seems useful here, as well.  Maybe factored into a separate function.
>>
>> The below implements this new idea, which indeed appears to work well: I
>> tested it and testsuite-wise seems essentially equivalent to what I posted
>> yesterday, besides a slightly worse error-recovery for the first issue in
>> cpp1z/decomp16.C: an additional 'no match for ‘operator=’' error.

I notice that we can improve error-recovery for decomp16.C by checking
for cp_error_declarator as well.

Tested x86_64-pc-linux-gnu, applying to trunkk.

[-- Attachment #2: err-dcl.diff --]
[-- Type: text/x-patch, Size: 1592 bytes --]

commit d06d67adb01349f4ba9ddf2572c3efcea86741f5
Author: Jason Merrill <jason@redhat.com>
Date:   Tue May 29 18:18:31 2018 -0400

    Improve error recovery for structured binding in condition.
    
            * parser.c (cp_parser_check_condition_declarator): Handle
            cp_error_declarator.

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index d17beb8f930..de090d42d8d 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -11594,10 +11594,13 @@ cp_parser_check_condition_declarator (cp_parser* parser,
                                      cp_declarator *declarator,
                                      location_t loc)
 {
-  if (function_declarator_p (declarator)
+  if (declarator == cp_error_declarator
+      || function_declarator_p (declarator)
       || declarator->kind == cdk_array)
     {
-      if (declarator->kind == cdk_array)
+      if (declarator == cp_error_declarator)
+	/* Already complained.  */;
+      else if (declarator->kind == cdk_array)
        error_at (loc, "condition declares an array");
       else
        error_at (loc, "condition declares a function");
diff --git a/gcc/testsuite/g++.dg/cpp1z/decomp16.C b/gcc/testsuite/g++.dg/cpp1z/decomp16.C
index dad69b89c08..7589c8015a5 100644
--- a/gcc/testsuite/g++.dg/cpp1z/decomp16.C
+++ b/gcc/testsuite/g++.dg/cpp1z/decomp16.C
@@ -8,7 +8,7 @@ void
 foo ()
 {
   auto [ a, b ] = A ();
-  for (; auto [ a, b ] = A (); )			// { dg-error "expected|no match" }
+  for (; auto [ a, b ] = A (); )			// { dg-error "expected" }
     ;
   for (; false; auto [ a, b ] = A ())			// { dg-error "expected" }
     ;