[C++ PATCH] Fix ICEs due to cp_parser_postfix_dot_deref_expression workaround (PR c++/84082)

[C++ PATCH] Fix ICEs due to cp_parser_postfix_dot_deref_expression workaround (PR c++/84082)

[Jakub Jelinek]

Hi!

In r245223 cp_parser_postfix_dot_deref_expression has been changed to
workaround some buggy template code with a pedwarn instead of error,
in r245440 Marek tweaked that by adding the && EXPR_P (postfix_expression)
because we really don't want to clear TREE_TYPE on OVERLOADs or on DECLs
that have incomplete type.

There are some expression types where we don't want to clear TREE_TYPE
either, like *CAST_EXPR, or VIEW_CONVERT_EXPR, these in various spots
assume they have non-NULL TREE_TYPE.

So, the following patch extends it by looking at the various
postfix_expression kinds and deciding what to do by tweaking kind.
Changing kind from DK_PEDWARN where it pedwarns + clears TREE_TYPE and scope
to DK_ERROR will result in an error + what we used to do before r245223,
i.e. scope = error_mark_node and postfix_expression = error_mark_node too.
Changing kind to DK_IGNORED will do what Marek's patch did, not diagnose
anything at that point, let other code diagnose incomplete type later on
or diagnose some other error.  For OVERLOAD that seems like a better
choice, not really sure about other cases.  For now the patch uses DK_ERROR
and thus what we used to do before r245223 for non-OVERLOAD.  At least
when not processing_template_decl I'd say it is better to error right away.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Or should I change the switch body to do other kind = overrides?

2018-01-29  Jakub Jelinek  <jakub@redhat.com>

	PR c++/84082
	* parser.c (cp_parser_postfix_dot_deref_expression): Don't clear
	TREE_TYPE in templates on expressions that don't allow NULL TREE_TYPE,
	use error instead of pedwarn in that case and set scope and
	postfix_expression to error_mark_node.

	* g++.dg/template/incomplete11.C: New test.
	* g++.dg/parse/crash67.C: Expect an incomplete type diagnostics too.

--- gcc/cp/parser.c.jj	2018-01-18 00:42:02.287016215 +0100
+++ gcc/cp/parser.c	2018-01-29 16:07:06.381822967 +0100
@@ -7450,9 +7450,7 @@ cp_parser_postfix_dot_deref_expression (
 		   (scope, current_class_type))))
 	{
 	  scope = complete_type (scope);
-	  if (!COMPLETE_TYPE_P (scope)
-	      /* Avoid clobbering e.g. OVERLOADs or DECLs.  */
-	      && EXPR_P (postfix_expression))
+	  if (!COMPLETE_TYPE_P (scope))
 	    {
 	      /* In a template, be permissive by treating an object expression
 		 of incomplete type as dependent (after a pedwarn).  */
@@ -7460,15 +7458,45 @@ cp_parser_postfix_dot_deref_expression (
 				   && MAYBE_CLASS_TYPE_P (scope)
 				   ? DK_PEDWARN
 				   : DK_ERROR);
-	      cxx_incomplete_type_diagnostic
-		(location_of (postfix_expression),
-		 postfix_expression, scope, kind);
-	      if (!MAYBE_CLASS_TYPE_P (scope))
-		return error_mark_node;
-	      if (processing_template_decl)
+
+	      switch (TREE_CODE (postfix_expression))
 		{
-		  dependent_p = true;
-		  scope = TREE_TYPE (postfix_expression) = NULL_TREE;
+		case CAST_EXPR:
+		case REINTERPRET_CAST_EXPR:
+		case CONST_CAST_EXPR:
+		case STATIC_CAST_EXPR:
+		case DYNAMIC_CAST_EXPR:
+		case IMPLICIT_CONV_EXPR:
+		case VIEW_CONVERT_EXPR:
+		  kind = DK_ERROR;
+		  break;
+		case OVERLOAD:
+		  /* Don't emit any diagnostic for OVERLOADs.  */
+		  kind = DK_IGNORED;
+		  break;
+		default:
+		  /* Avoid clobbering e.g. DECLs.  */
+		  if (!EXPR_P (postfix_expression))
+		    kind = DK_ERROR;
+		  break;
+		}
+	      if (kind != DK_IGNORED)
+		{
+		  location_t exploc = location_of (postfix_expression);
+		  cxx_incomplete_type_diagnostic (exploc, postfix_expression,
+						  scope, kind);
+		  if (!MAYBE_CLASS_TYPE_P (scope))
+		    return error_mark_node;
+		  if (kind == DK_ERROR)
+		    {
+		      scope = error_mark_node;
+		      postfix_expression = error_mark_node;
+		    }
+		  else if (processing_template_decl)
+		    {
+		      dependent_p = true;
+		      scope = TREE_TYPE (postfix_expression) = NULL_TREE;
+		    }
 		}
 	    }
 	}
--- gcc/testsuite/g++.dg/template/incomplete11.C.jj	2018-01-29 16:01:32.762637830 +0100
+++ gcc/testsuite/g++.dg/template/incomplete11.C	2018-01-29 16:00:34.066605257 +0100
@@ -0,0 +1,10 @@
+// PR c++/84082
+// { dg-do compile }
+// { dg-options "" }
+
+struct A;
+
+template<typename> void foo()
+{
+  static int a[A().operator=(A())];	// { dg-error "invalid use of incomplete type 'struct A'" }
+}
--- gcc/testsuite/g++.dg/parse/crash67.C.jj	2017-11-06 17:23:33.849713317 +0100
+++ gcc/testsuite/g++.dg/parse/crash67.C	2018-01-29 23:46:19.697231914 +0100
@@ -3,4 +3,4 @@
 
 class x0;
 template <x1> x2() {  // { dg-error "declared|type" }
-x0 x3 = x3.  // { dg-error "expected" }
+x0 x3 = x3.  // { dg-error "expected|incomplete type" }

	Jakub

Re: [C++ PATCH] Fix ICEs due to cp_parser_postfix_dot_deref_expression workaround (PR c++/84082)

[Jason Merrill]

On Mon, Jan 29, 2018 at 6:29 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> In r245223 cp_parser_postfix_dot_deref_expression has been changed to
> workaround some buggy template code with a pedwarn instead of error,
> in r245440 Marek tweaked that by adding the && EXPR_P (postfix_expression)
> because we really don't want to clear TREE_TYPE on OVERLOADs or on DECLs
> that have incomplete type.
>
> There are some expression types where we don't want to clear TREE_TYPE
> either, like *CAST_EXPR, or VIEW_CONVERT_EXPR, these in various spots
> assume they have non-NULL TREE_TYPE.
>
> So, the following patch extends it by looking at the various
> postfix_expression kinds and deciding what to do by tweaking kind.
> Changing kind from DK_PEDWARN where it pedwarns + clears TREE_TYPE and scope
> to DK_ERROR will result in an error + what we used to do before r245223,
> i.e. scope = error_mark_node and postfix_expression = error_mark_node too.
> Changing kind to DK_IGNORED will do what Marek's patch did, not diagnose
> anything at that point, let other code diagnose incomplete type later on
> or diagnose some other error.  For OVERLOAD that seems like a better
> choice, not really sure about other cases.  For now the patch uses DK_ERROR
> and thus what we used to do before r245223 for non-OVERLOAD.  At least
> when not processing_template_decl I'd say it is better to error right away.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> Or should I change the switch body to do other kind = overrides?

I think we want to avoid clobbering NON_LVALUE_EXPR location wrappers, too.

This has gotten large enough that it should break out into its own function.

OK with those changes.

Jason

[C++ PATCH] Fix ICEs due to cp_parser_postfix_dot_deref_expression workaround (PR c++/84082, take 2)

[Jakub Jelinek]

On Fri, Feb 02, 2018 at 03:13:21PM -0500, Jason Merrill wrote:
> > Or should I change the switch body to do other kind = overrides?
> 
> I think we want to avoid clobbering NON_LVALUE_EXPR location wrappers, too.
> 
> This has gotten large enough that it should break out into its own function.
> 
> OK with those changes.

So like this?  Bootstrapped/regtested on x86_64-linux and i686-linux.

2018-02-06  Jakub Jelinek  <jakub@redhat.com>

	PR c++/84082
	* parser.c (cp_parser_dot_deref_incomplete): New function.
	(cp_parser_postfix_dot_deref_expression): Use it.

	* g++.dg/template/incomplete11.C: New test.
	* g++.dg/parse/crash67.C: Expect an incomplete type diagnostics too.

--- gcc/cp/parser.c.jj	2018-01-31 19:07:35.401199026 +0100
+++ gcc/cp/parser.c	2018-02-03 19:08:37.828664789 +0100
@@ -7387,6 +7387,63 @@ cp_parser_postfix_open_square_expression
   return postfix_expression;
 }
 
+/* A subroutine of cp_parser_postfix_dot_deref_expression.  Handle dot
+   dereference of incomplete type, returns true if error_mark_node should
+   be returned from caller, otherwise adjusts *SCOPE, *POSTFIX_EXPRESSION
+   and *DEPENDENT_P.  */
+
+bool
+cp_parser_dot_deref_incomplete (tree *scope, cp_expr *postfix_expression,
+				bool *dependent_p)
+{
+  /* In a template, be permissive by treating an object expression
+     of incomplete type as dependent (after a pedwarn).  */
+  diagnostic_t kind = (processing_template_decl
+		       && MAYBE_CLASS_TYPE_P (*scope) ? DK_PEDWARN : DK_ERROR);
+
+  switch (TREE_CODE (*postfix_expression))
+    {
+    case CAST_EXPR:
+    case REINTERPRET_CAST_EXPR:
+    case CONST_CAST_EXPR:
+    case STATIC_CAST_EXPR:
+    case DYNAMIC_CAST_EXPR:
+    case IMPLICIT_CONV_EXPR:
+    case VIEW_CONVERT_EXPR:
+      kind = DK_ERROR;
+      break;
+    case NON_LVALUE_EXPR:
+      if (location_wrapper_p (*postfix_expression))
+	kind = DK_ERROR;
+      break;
+    case OVERLOAD:
+      /* Don't emit any diagnostic for OVERLOADs.  */
+      kind = DK_IGNORED;
+      break;
+    default:
+      /* Avoid clobbering e.g. DECLs.  */
+      if (!EXPR_P (*postfix_expression))
+	kind = DK_ERROR;
+      break;
+    }
+
+  if (kind == DK_IGNORED)
+    return false;
+
+  location_t exploc = location_of (*postfix_expression);
+  cxx_incomplete_type_diagnostic (exploc, *postfix_expression, *scope, kind);
+  if (!MAYBE_CLASS_TYPE_P (*scope))
+    return true;
+  if (kind == DK_ERROR)
+    *scope = *postfix_expression = error_mark_node;
+  else if (processing_template_decl)
+    {
+      *dependent_p = true;
+      *scope = TREE_TYPE (*postfix_expression) = NULL_TREE;
+    }
+  return false;
+}
+
 /* A subroutine of cp_parser_postfix_expression that also gets hijacked
    by cp_parser_builtin_offsetof.  We're looking for
 
@@ -7451,26 +7508,9 @@ cp_parser_postfix_dot_deref_expression (
 	{
 	  scope = complete_type (scope);
 	  if (!COMPLETE_TYPE_P (scope)
-	      /* Avoid clobbering e.g. OVERLOADs or DECLs.  */
-	      && EXPR_P (postfix_expression))
-	    {
-	      /* In a template, be permissive by treating an object expression
-		 of incomplete type as dependent (after a pedwarn).  */
-	      diagnostic_t kind = (processing_template_decl
-				   && MAYBE_CLASS_TYPE_P (scope)
-				   ? DK_PEDWARN
-				   : DK_ERROR);
-	      cxx_incomplete_type_diagnostic
-		(location_of (postfix_expression),
-		 postfix_expression, scope, kind);
-	      if (!MAYBE_CLASS_TYPE_P (scope))
-		return error_mark_node;
-	      if (processing_template_decl)
-		{
-		  dependent_p = true;
-		  scope = TREE_TYPE (postfix_expression) = NULL_TREE;
-		}
-	    }
+	      && cp_parser_dot_deref_incomplete (&scope, &postfix_expression,
+						 &dependent_p))
+	    return error_mark_node;
 	}
 
       if (!dependent_p)
--- gcc/testsuite/g++.dg/template/incomplete11.C.jj	2018-02-03 17:56:10.536083614 +0100
+++ gcc/testsuite/g++.dg/template/incomplete11.C	2018-02-03 17:56:10.536083614 +0100
@@ -0,0 +1,10 @@
+// PR c++/84082
+// { dg-do compile }
+// { dg-options "" }
+
+struct A;
+
+template<typename> void foo()
+{
+  static int a[A().operator=(A())];	// { dg-error "invalid use of incomplete type 'struct A'" }
+}
--- gcc/testsuite/g++.dg/parse/crash67.C.jj	2018-01-31 19:07:33.571220455 +0100
+++ gcc/testsuite/g++.dg/parse/crash67.C	2018-02-03 17:56:10.536083614 +0100
@@ -3,4 +3,4 @@
 
 class x0;
 template <x1> x2() {  // { dg-error "declared|type" }
-x0 x3 = x3.  // { dg-error "expected" }
+x0 x3 = x3.  // { dg-error "expected|incomplete type" }


	Jakub

Re: [C++ PATCH] Fix ICEs due to cp_parser_postfix_dot_deref_expression workaround (PR c++/84082, take 2)

[Jason Merrill]

On Tue, Feb 6, 2018 at 3:28 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Feb 02, 2018 at 03:13:21PM -0500, Jason Merrill wrote:
>> > Or should I change the switch body to do other kind = overrides?
>>
>> I think we want to avoid clobbering NON_LVALUE_EXPR location wrappers, too.
>>
>> This has gotten large enough that it should break out into its own function.
>>
>> OK with those changes.
>
> So like this?  Bootstrapped/regtested on x86_64-linux and i686-linux.
>
> +    case NON_LVALUE_EXPR:
> +      if (location_wrapper_p (*postfix_expression))

I don't think we need this test, as NON_LVALUE_EXPR should only appear
as a location wrapper.  OK with it removed.

Jason