diff mbox

[C++] PR 58633

Message ID 525459D9.1020601@oracle.com
State New
Headers show

Commit Message

Paolo Carlini Oct. 8, 2013, 7:15 p.m. UTC 
Hi,

in this ICE on valid, 4.7/4.8/4.9 Regression, the ICE happens in the 
second half of cp_parser_decltype_expr, when 
cp_parser_abort_tentative_parse is called in an inconstent status: 
tentative parsing is committed and no errors. The reason is the 
following: in its main loop cp_parser_postfix_expression calls 
cp_parser_postfix_dot_deref_expression which in turns calls 
cp_parser_pseudo_destructor_name and everything appears to be fine, thus 
the latter commits the tentative parse, but member_access_only_p is 
*true* in the first half of cp_parser_decltype_expr and even if 
cp_parser_postfix_expression later also finds valid () following the 
pseudo dtor expression has to return error_mark_node anyway.

A possible way to resolve the inconsistency is propagating 
member_access_only_p past cp_parser_postfix_expression to 
cp_parser_pseudo_destructor_name and in the latter not committing the 
tentative parse when the flag is true.

Tested x86_64-linux.

Thanks,
Paolo.

///////////////////////////
/cp
2013-10-08  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/58633
	* parser.c (cp_parser_postfix_dot_deref_expression,
	cp_parser_pseudo_destructor_name): Add bool parameter.
	(cp_parser_postfix_expression, cp_parser_builtin_offsetof):
	Adjust.

/testsuite
2013-10-08  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/58633
	* g++.dg/cpp0x/decltype57.C: New.

Comments

Paolo Carlini Oct. 8, 2013, 11:03 p.m. UTC | #1 
.. a curiosity: the cp_parser_commit_to_tentative_parse at the end of 
cp_parser_pseudo_destructor_name, which didn't exist in 4.6.x and we can 
consider the root of this issue, is also my fault:

     http://gcc.gnu.org/ml/gcc-patches/2011-05/msg02246.html

 From a different angle, I'm happy of the outcome of this detective 
work, because it means that the parser_commit isn't there for 
correctness: not performing it in some cases shouldn't be a big issue.

Paolo.
Jason Merrill Oct. 9, 2013, 1:56 p.m. UTC | #2 
On 10/08/2013 07:03 PM, Paolo Carlini wrote:
> .. a curiosity: the cp_parser_commit_to_tentative_parse at the end of
> cp_parser_pseudo_destructor_name, which didn't exist in 4.6.x and we can
> consider the root of this issue, is also my fault:
>
>      http://gcc.gnu.org/ml/gcc-patches/2011-05/msg02246.html
>
>  From a different angle, I'm happy of the outcome of this detective
> work, because it means that the parser_commit isn't there for
> correctness: not performing it in some cases shouldn't be a big issue.

Yeah.  I don't understand why cp_parser_commit_to_tentative_parse 
commits all levels, rather than just the current one as the comment 
seems to suggest.  Mark, do you remember anything about that decision?

Jason
diff mbox

Patch

Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 203274)
+++ cp/parser.c	(working copy)
@@ -1861,13 +1861,13 @@  static tree cp_parser_postfix_expression
 static tree cp_parser_postfix_open_square_expression
   (cp_parser *, tree, bool, bool);
 static tree cp_parser_postfix_dot_deref_expression
-  (cp_parser *, enum cpp_ttype, tree, bool, cp_id_kind *, location_t);
+  (cp_parser *, enum cpp_ttype, tree, bool, bool, cp_id_kind *, location_t);
 static vec<tree, va_gc> *cp_parser_parenthesized_expression_list
   (cp_parser *, int, bool, bool, bool *);
 /* Values for the second parameter of cp_parser_parenthesized_expression_list.  */
 enum { non_attr = 0, normal_attr = 1, id_attr = 2 };
 static void cp_parser_pseudo_destructor_name
-  (cp_parser *, tree, tree *, tree *);
+  (cp_parser *, tree, tree *, tree *, bool);
 static tree cp_parser_unary_expression
   (cp_parser *, bool, bool, cp_id_kind *);
 static enum tree_code cp_parser_unary_operator
@@ -6028,7 +6028,9 @@  cp_parser_postfix_expression (cp_parser *parser, b
 	  postfix_expression
 	    = cp_parser_postfix_dot_deref_expression (parser, token->type,
 						      postfix_expression,
-						      false, &idk, loc);
+						      false,
+						      member_access_only_p,
+						      &idk, loc);
 
           is_member_access = true;
 	  break;
@@ -6261,7 +6263,9 @@  static tree
 cp_parser_postfix_dot_deref_expression (cp_parser *parser,
 					enum cpp_ttype token_type,
 					tree postfix_expression,
-					bool for_offsetof, cp_id_kind *idk,
+					bool for_offsetof,
+					bool member_access_only_p,
+					cp_id_kind *idk,
 					location_t location)
 {
   tree name;
@@ -6337,7 +6341,7 @@  cp_parser_postfix_dot_deref_expression (cp_parser
       /* Parse the pseudo-destructor-name.  */
       s = NULL_TREE;
       cp_parser_pseudo_destructor_name (parser, postfix_expression,
-					&s, &type);
+					&s, &type, member_access_only_p);
       if (dependent_p
 	  && (cp_parser_error_occurred (parser)
 	      || !SCALAR_TYPE_P (type)))
@@ -6610,7 +6614,8 @@  static void
 cp_parser_pseudo_destructor_name (cp_parser* parser,
 				  tree object,
 				  tree* scope,
-				  tree* type)
+				  tree* type,
+				  bool member_access_only_p)
 {
   bool nested_name_specifier_p;
 
@@ -6692,7 +6697,15 @@  cp_parser_pseudo_destructor_name (cp_parser* parse
   cp_parser_require (parser, CPP_COMPL, RT_COMPL);
 
   /* Once we see the ~, this has to be a pseudo-destructor.  */
-  if (!processing_template_decl && !cp_parser_error_occurred (parser))
+  if (!processing_template_decl && !cp_parser_error_occurred (parser)
+      /* ... but we don't want to prematurely commit when only
+	 member access expressions are allowed.  This happens
+	 when parsing a decltype, because pseudo destructor
+	 calls (5.2.4) are handled by cp_parser_decltype_expr
+	 via the final cp_parser_expression (which eventually
+	 calls cp_parser_postfix_expression with a false
+	 member_access_only_p) (c++/58633).  */
+      && !member_access_only_p)
     cp_parser_commit_to_tentative_parse (parser);
 
   /* Look for the type-name again.  We are not responsible for
@@ -8258,8 +8271,9 @@  cp_parser_builtin_offsetof (cp_parser *parser)
                             tf_warning_or_error);
 
   /* Parse the offsetof-member-designator.  We begin as if we saw "expr->".  */
-  expr = cp_parser_postfix_dot_deref_expression (parser, CPP_DEREF, expr,
-						 true, &dummy, token->location);
+  expr = cp_parser_postfix_dot_deref_expression (parser, CPP_DEREF, expr, true,
+						 /*member_access_only_p=*/false,
+						 &dummy, token->location);
   while (true)
     {
       token = cp_lexer_peek_token (parser->lexer);
@@ -8280,9 +8294,9 @@  cp_parser_builtin_offsetof (cp_parser *parser)
 	case CPP_DOT:
 	  /* offsetof-member-designator "." identifier */
 	  cp_lexer_consume_token (parser->lexer);
-	  expr = cp_parser_postfix_dot_deref_expression (parser, CPP_DOT,
-							 expr, true, &dummy,
-							 token->location);
+	  expr = (cp_parser_postfix_dot_deref_expression
+		  (parser, CPP_DOT, expr, true, /*member_access_only_p=*/false,
+		   &dummy, token->location));
 	  break;
 
 	case CPP_CLOSE_PAREN:
Index: testsuite/g++.dg/cpp0x/decltype57.C
===================================================================
--- testsuite/g++.dg/cpp0x/decltype57.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/decltype57.C	(working copy)
@@ -0,0 +1,8 @@ 
+// PR c++/58633
+// { dg-do compile { target c++11 } }
+
+void foo(int i)
+{
+  typedef int I;
+  decltype(i.I::~I())* p;
+}