On 3/25/20 12:17 PM, Patrick Palka wrote:
This PR reports that the requires-expression in
auto f = [] { };
static_assert(requires { f(); });
erroneously evaluates to false. The way we end up evaluating to false goes as
follows. During the parsing of the requires-expression, we call
finish_call_expr from cp_parser_requires_expression with the arguments
fn = VIEW_CONVERT_EXPR<struct ._anon_0>(f);
args = {}
which does the full processing of the call (since we're not inside a template)
and returns
<lambda()>::operator() (&f);
Later, when evaluating the requires-expression, we call finish_call_expr again,
this time from tsubst_expr from satisfy_atom, with the arguments
fn = operator()
args = { &f }
(which, as expected, correspond to the CALL_EXPR returned by finish_call_expr
the first time). But this time, finish_call_expr returns error_mark_node
because
it doesn't expect to see an explicit 'this' argument in the args array, treating
it instead as a user-written argument which causes the only candidate function
to be discarded. This causes the requires-expression to evaluate to false.
In short, it seems finish_call_expr is not idempotent on certain inputs when
!processing_template_decl. Assuming this idempotency issue is not specific to
finish_call_expr, it seems that the safest thing to do is to avoid doing full
semantic processing twice when parsing and evaluating a requires-expression that
lives outside of a template definition.
Absolutely. We shouldn't call tsubst_expr on non-template trees.
This patch achieves this by temporarily setting processing_template_decl to
non-zero when parsing the body of a requires-expression. This way, full
semantic processing will always be done only during evaluation and not during
parsing.
Hmm, interesting approach, but I think the standard requires us to treat
requires-expressions outside of template context like normal
non-template code: "[Note: If a requires-expression contains invalid
types or expressions in its requirements, and it does not appear within
the declaration of a templated entity, then the program is ill-formed.
--end note]"
So I think better to avoid the tsubst_expr later, either by immediately
evaluating the REQUIRES_EXPR or marking the ATOMIC_CONSTR. We could do
that by immediately resolving the requires-expression in non-template
context, or by marking it up somehow to prevent the substitution.
I notice that we currently fail to handle requires-expressions in
regular expression context:
int main() { return requires { 42; }; } // ICE
Testing is in progress. Does this look like the right solution?
gcc/cp/ChangeLog:
PR c++/94252
* parser.c (cp_parser_requires_expression): Always parse the requirement
body as if we're processing a template, by temporarily setting
processing_template_decl to non-zero.
* semantics.c (finish_static_assert): Also explain an assertion failure
when the condition is a REQUIRES_EXPR.
gcc/testsuite/ChangeLog:
PR c++/94252
* g++.dg/concepts/diagnostic7.C: New test.
* g++.dg/concepts/pr94252.C: New test.
---
gcc/cp/parser.c | 7 ++++++-
gcc/cp/semantics.c | 6 ++++--
gcc/testsuite/g++.dg/concepts/diagnostic7.C | 12 ++++++++++++
gcc/testsuite/g++.dg/concepts/pr94252.C | 14 ++++++++++++++
4 files changed, 36 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic7.C
create mode 100644 gcc/testsuite/g++.dg/concepts/pr94252.C
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 03ecd7838f6..14a22b85318 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -27689,7 +27689,12 @@ cp_parser_requires_expression (cp_parser *parser)
else
parms = NULL_TREE;
- /* Parse the requirement body. */
+ /* Always parse the requirement body as if we're inside a template so that
+ we always do full semantic processing only during evaluation of this
+ requires-expression and not also now during parsing. */
+ processing_template_decl_sentinel ptds;
+ if (!processing_template_decl)
+ processing_template_decl = 1;
reqs = cp_parser_requirement_body (parser);
if (reqs == error_mark_node)
return error_mark_node;
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index bcb2e72fbb5..e998b373af4 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -9691,8 +9691,10 @@ finish_static_assert (tree condition, tree message,
location_t location,
error ("static assertion failed: %s",
TREE_STRING_POINTER (message));
- /* Actually explain the failure if this is a concept check. */
- if (concept_check_p (orig_condition))
+ /* Actually explain the failure if this is a concept check or a
+ requires-expression. */
+ if (concept_check_p (orig_condition)
+ || TREE_CODE (orig_condition) == REQUIRES_EXPR)
diagnose_constraints (location, orig_condition, NULL_TREE);
}
else if (condition && condition != error_mark_node)
diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic7.C
b/gcc/testsuite/g++.dg/concepts/diagnostic7.C
new file mode 100644
index 00000000000..f78e9bb8240
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/diagnostic7.C
@@ -0,0 +1,12 @@
+// { dg-do compile { target c++2a } }
+
+template<typename A, typename B>
+ concept same_as = __is_same(A, B); // { dg-message ".void. is not the same as
.int." }
+
+void f();
+
+static_assert(requires { { f() } noexcept -> same_as<int>; });
+// { dg-error "static assertion failed" "" { target *-*-* } .-1 }
+// { dg-message "not .noexcept." "" { target *-*-* } .-2 }
+// { dg-message "return-type-requirement" "" { target *-*-* } .-3 }
+// { dg-error "does not satisfy placeholder constraints" "" { target *-*-* }
.-4 }
diff --git a/gcc/testsuite/g++.dg/concepts/pr94252.C
b/gcc/testsuite/g++.dg/concepts/pr94252.C
new file mode 100644
index 00000000000..7b93997363a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr94252.C
@@ -0,0 +1,14 @@
+// PR c++/94252
+// { dg-do compile { target c++2a } }
+
+auto f = []{ return 0; };
+static_assert(requires { f(); });
+
+template<typename A, typename B>
+ concept same_as = __is_same(A, B);
+
+struct S { int f(int) noexcept; };
+static_assert(requires(S o, int i) {
+ o.f(i);
+ { o.f(i) } noexcept -> same_as<int>;
+});
