Patchwork [C++] PR 51908

login
register
mail settings
Submitter Paolo Carlini
Date June 5, 2013, 3:45 p.m.
Message ID <51AF5D07.40408@oracle.com>
Download mbox | patch
Permalink /patch/249130/
State New
Headers show

Comments

Paolo Carlini - June 5, 2013, 3:45 p.m.
Hi,

On 06/05/2013 04:19 PM, Jason Merrill wrote:
> On 06/05/2013 09:19 AM, Jason Merrill wrote:
>> In any case, the commit doesn't seem like the problem.
>
> Oh, I see, of course it is.  The problem is that it commits to all 
> levels, so that if we happen to be in a nested tentative parse we 
> can't commit to the inner one without committing to the outer one, 
> which might not be appropriate.  This seems like a basic flaw in our 
> tentative parsing mechanism.
Uhmmm.
> What happens if we disable that commit entirely?
Not a disaster, I'm attaching a breakdown of the regressions. 
Essentially no accept invalid or reject valid, but in some cases the 
quality of the diagnostic worsen a lot and/or becomes more verbose.

Now, something else I noticed a few days ago is that for C-style casts 
things work fine in any case: the difference being that in that case we 
set parser->in_type_id_in_expr_p to true, something we don't do for 
static_cast (and the other C++ casts) in cp_parser_postfix_expression. 
If I do that, then all the testcases we are considering pass with no 
regressions in the testsuite. Is it something we could do? Patch draft 
attached.

Thanks,
Paolo.

///////////////////////
FAIL: g++.dg/other/pr39060.C -std=c++98  (test for errors, line 19)
FAIL: g++.dg/other/pr39060.C -std=c++98 (test for excess errors)
FAIL: g++.dg/other/pr39060.C -std=c++11  (test for errors, line 19)
FAIL: g++.dg/other/pr39060.C -std=c++11 (test for excess errors)
FAIL: g++.dg/parse/crash50.C -std=c++98  (test for errors, line 5)
FAIL: g++.dg/parse/crash50.C -std=c++98  (test for errors, line 9)
FAIL: g++.dg/parse/crash50.C -std=c++98 (test for excess errors)
FAIL: g++.dg/parse/crash50.C -std=c++11  (test for errors, line 5)
FAIL: g++.dg/parse/crash50.C -std=c++11  (test for errors, line 9)
FAIL: g++.dg/parse/crash50.C -std=c++11 (test for excess errors)
FAIL: g++.dg/parse/error6.C -std=gnu++98  (test for errors, line 4)
FAIL: g++.dg/parse/error6.C -std=gnu++98  (test for errors, line 5)
FAIL: g++.dg/parse/error6.C -std=gnu++98 (test for excess errors)
FAIL: g++.dg/parse/error6.C -std=gnu++11  (test for errors, line 4)
FAIL: g++.dg/parse/error6.C -std=gnu++11  (test for errors, line 5)
FAIL: g++.dg/parse/error6.C -std=gnu++11 (test for excess errors)
FAIL: g++.dg/parse/error7.C -std=gnu++98  (test for errors, line 5)
FAIL: g++.dg/parse/error7.C -std=gnu++98 (test for excess errors)
FAIL: g++.dg/parse/error7.C -std=gnu++11  (test for errors, line 5)
FAIL: g++.dg/parse/error7.C -std=gnu++11 (test for excess errors)
FAIL: g++.dg/parse/varmod1.C -std=c++98 (test for excess errors)
FAIL: g++.dg/parse/varmod1.C -std=c++11 (test for excess errors)
FAIL: g++.dg/template/crash77.C -std=c++98  (test for errors, line 3)
FAIL: g++.dg/template/crash77.C -std=c++11  (test for errors, line 3)
FAIL: g++.old-deja/g++.brendan/crash61.C -std=c++98 (test for excess errors)
FAIL: g++.old-deja/g++.brendan/crash61.C -std=c++11 (test for excess errors)
Jason Merrill - June 5, 2013, 5:16 p.m.
On 06/05/2013 11:45 AM, Paolo Carlini wrote:
> Now, something else I noticed a few days ago is that for C-style casts
> things work fine in any case: the difference being that in that case we
> set parser->in_type_id_in_expr_p to true, something we don't do for
> static_cast (and the other C++ casts) in cp_parser_postfix_expression.
> If I do that, then all the testcases we are considering pass with no
> regressions in the testsuite. Is it something we could do? Patch draft
> attached.

OK.

Jason

Patch

Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 199690)
+++ cp/parser.c	(working copy)
@@ -5546,6 +5546,7 @@  cp_parser_postfix_expression (cp_parser *parser, b
 	tree type;
 	tree expression;
 	const char *saved_message;
+	bool saved_in_type_id_in_expr_p;
 
 	/* All of these can be handled in the same way from the point
 	   of view of parsing.  Begin by consuming the token
@@ -5560,7 +5561,10 @@  cp_parser_postfix_expression (cp_parser *parser, b
 	/* Look for the opening `<'.  */
 	cp_parser_require (parser, CPP_LESS, RT_LESS);
 	/* Parse the type to which we are casting.  */
+	saved_in_type_id_in_expr_p = parser->in_type_id_in_expr_p;
+	parser->in_type_id_in_expr_p = true;
 	type = cp_parser_type_id (parser);
+	parser->in_type_id_in_expr_p = saved_in_type_id_in_expr_p;
 	/* Look for the closing `>'.  */
 	cp_parser_require (parser, CPP_GREATER, RT_GREATER);
 	/* Restore the old message.  */
Index: testsuite/g++.dg/cpp0x/decltype54.C
===================================================================
--- testsuite/g++.dg/cpp0x/decltype54.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/decltype54.C	(working copy)
@@ -0,0 +1,26 @@ 
+// PR c++/51908
+// { dg-do compile { target c++11 } }
+
+struct foo1
+{
+  template <typename Ret, typename... Args>
+  operator decltype(static_cast<Ret (*)(Args...)>(nullptr)) () const;
+};
+
+struct foo2
+{
+  template <typename Ret, typename... Args>
+  operator decltype(static_cast<Ret (*)(Args... args)>(nullptr)) () const;
+};
+
+struct foo3
+{
+  template <typename Ret, typename Arg>
+  operator decltype(static_cast<Ret (*)(Arg)>(nullptr)) () const;
+};
+
+struct foo4
+{
+  template <typename Ret, typename Arg>
+  operator decltype(static_cast<Ret (*)(Arg arg)>(nullptr)) () const;
+};