Re: [C++ Patch] PR 84588 ("[8 Regression] internal compiler error: Segmentation fault (contains_struct_check())") (Take 2)
Paolo Carlini
paolo.carlini@oracle.com
Sun May 20 16:10:00 GMT 2018
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.
/////////////////////
-------------- next part --------------
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" }
;
}
More information about the Gcc-patches
mailing list