| Message ID | c26f5fbb-9358-1ab8-7086-1ab667e46f18@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | [C++,Patch/RFC] PR 88987 ("[9 Regression] ICE: unexpected expression '(bool)sm' of kind implicit_conv_expr") | expand |
On 2/26/19 5:42 AM, Paolo Carlini wrote: > Hi, > > the issue is rather easy to explain: after Alexandre' change in > r266874, which simplified the condition at the beginning of > build_noexcept_spec to evaluate early all the expressions which aren't > deferred or value-dependent, we obviously ICE during error-recovery on > the new testcase because an expression which isn't a potential constant > filters through and cxx_constant_value can't handle it. We can avoid > this in various ways - for example by adding a gate && > potential_rvalue_constant_expression_p (expr) in the condition at the > beginning of build_noexcept_spec and adjust/remove the final gcc_assert > (which is already a bit obsolete wrt Alexandre' change). Or we can > handle this earlier, in cp_parser_noexcept_specification_opt - in > complete analogy, with, say, cp_parser_initializer_list - thus don't let > through those expressions at all (possible variant: set expr = > error_mark_node), which has the advantage of avoiding a duplicate > potential_rvalue_constant_expression call (note: in the parser > build_no_except_spec is called only by > cp_parser_noexcept_specification_opt) > > All those variants pass the testsuite on x86_64-linux. What happens if 'sm' is constexpr? Jason
Hi, On 26/02/19 15:33, Jason Merrill wrote: > On 2/26/19 5:42 AM, Paolo Carlini wrote: >> Hi, >> >> the issue is rather easy to explain: after Alexandre' change in >> r266874, which simplified the condition at the beginning of >> build_noexcept_spec to evaluate early all the expressions which >> aren't deferred or value-dependent, we obviously ICE during >> error-recovery on the new testcase because an expression which isn't >> a potential constant filters through and cxx_constant_value can't >> handle it. We can avoid this in various ways - for example by adding >> a gate && potential_rvalue_constant_expression_p (expr) in the >> condition at the beginning of build_noexcept_spec and adjust/remove >> the final gcc_assert (which is already a bit obsolete wrt Alexandre' >> change). Or we can handle this earlier, in >> cp_parser_noexcept_specification_opt - in complete analogy, with, >> say, cp_parser_initializer_list - thus don't let through those >> expressions at all (possible variant: set expr = error_mark_node), >> which has the advantage of avoiding a duplicate >> potential_rvalue_constant_expression call (note: in the parser >> build_no_except_spec is called only by >> cp_parser_noexcept_specification_opt) >> >> All those variants pass the testsuite on x86_64-linux. > What happens if 'sm' is constexpr? Well, if 'sm' is constexpr we accept the snippet, as we should, AFAICS. This is true for all my tentative patches, I think, certainly for what I posted. Note, this is supposed to be only an error-recovery issue. Paolo.
On 2/26/19 10:22 AM, Paolo Carlini wrote: > Hi, > > On 26/02/19 15:33, Jason Merrill wrote: >> On 2/26/19 5:42 AM, Paolo Carlini wrote: >>> Hi, >>> >>> the issue is rather easy to explain: after Alexandre' change in >>> r266874, which simplified the condition at the beginning of >>> build_noexcept_spec to evaluate early all the expressions which >>> aren't deferred or value-dependent, we obviously ICE during >>> error-recovery on the new testcase because an expression which isn't >>> a potential constant filters through and cxx_constant_value can't >>> handle it. We can avoid this in various ways - for example by adding >>> a gate && potential_rvalue_constant_expression_p (expr) in the >>> condition at the beginning of build_noexcept_spec and adjust/remove >>> the final gcc_assert (which is already a bit obsolete wrt Alexandre' >>> change). Or we can handle this earlier, in >>> cp_parser_noexcept_specification_opt - in complete analogy, with, >>> say, cp_parser_initializer_list - thus don't let through those >>> expressions at all (possible variant: set expr = error_mark_node), >>> which has the advantage of avoiding a duplicate >>> potential_rvalue_constant_expression call (note: in the parser >>> build_no_except_spec is called only by >>> cp_parser_noexcept_specification_opt) >>> >>> All those variants pass the testsuite on x86_64-linux. >> What happens if 'sm' is constexpr? > > Well, if 'sm' is constexpr we accept the snippet, as we should, AFAICS. > This is true for all my tentative patches, I think, certainly for what I > posted. Yes, we should. Then the patch is OK. Jason
Index: cp/parser.c =================================================================== --- cp/parser.c (revision 269187) +++ cp/parser.c (working copy) @@ -25143,7 +25143,17 @@ cp_parser_noexcept_specification_opt (cp_parser* p parser->type_definition_forbidden_message = G_("types may not be defined in an exception-specification"); - expr = cp_parser_constant_expression (parser); + bool non_constant_p; + expr + = cp_parser_constant_expression (parser, + /*allow_non_constant=*/true, + &non_constant_p); + if (non_constant_p + && !require_potential_rvalue_constant_expression (expr)) + { + expr = NULL_TREE; + return_cond = true; + } /* Restore the saved message. */ parser->type_definition_forbidden_message = saved_message; Index: testsuite/g++.dg/cpp0x/pr88987.C =================================================================== --- testsuite/g++.dg/cpp0x/pr88987.C (nonexistent) +++ testsuite/g++.dg/cpp0x/pr88987.C (working copy) @@ -0,0 +1,9 @@ +// { dg-do compile { target c++11 } } + +int sm; + +template <typename T> T +pk () noexcept (sm) // { dg-error "constant expression" } +{ + return 0; +}