Tested x86_64-pc-linux-gnu, applying to trunk.
-- 8< --
Playing with using contract_assert for libstdc++ asserts led me to rework
how they are handled in constant evaluation.
The main change is that instead of always diagnosing under
cxx_eval_outermost_constant_expr, we diagnose during evaluation if we're
diagnosing. This means that the diagnostics now get the cx_call_context,
and (if terminating) we stop evaluating at that point.
If not diagnosing, but we saw a violation in an otherwise-constant
evaluation, we evaluate again to get the diagnostic in its call context.
And to diagnose multiple observes.
This approach also fixes non-const1.C, where we were wrongly complaining
that the predicate is non-constant; since the whole initialization is
non-constant, under [expr.const.init]/4.1 we shouldn't consider assertions
yet.
This also properly gets a diagnostic that an assertion fails in an
initialization that is otherwise constant (p17-SMF*).
Also for better context, I changed check_for_failed_contracts to
1) use the location of the predicate, not the keyword.
2) use cxx_constant_value or diagnose_failing_condition to explain
what went wrong.
This also works around an issue with maybe_unwind_expanded_macro_loc firing
repeatedly in a single diagnostic group--which would be good to fix in
general, but that's not this patch.
PR c++/119497
gcc/cp/ChangeLog:
* constexpr.cc (check_for_failed_contracts): Add context.
(cxx_eval_constant_expression): Refactor contract handling.
Diagnose now if diagnosing.
(cxx_eval_outermost_constant_expr): Re-evaluate to diagnose
contract violations if otherwise constant.
gcc/testsuite/ChangeLog:
* g++.dg/contracts/cpp26/basic.contract.eval.p17-SMF-post.C
* g++.dg/contracts/cpp26/basic.contract.eval.p17-SMF-pre.C
* g++.dg/contracts/cpp26/basic.contract.eval.p17-SMF2.C
* g++.dg/contracts/cpp26/basic.contract.eval.p7.3.C
* g++.dg/contracts/cpp26/callerside-checks/ctor.C: Add diagnostic.
* g++.dg/contracts/cpp26/non-const1.C: New test.
---
gcc/cp/constexpr.cc | 130 ++++++++++++------
.../cpp26/basic.contract.eval.p17-SMF-post.C | 2 +-
.../cpp26/basic.contract.eval.p17-SMF-pre.C | 2 +-
.../cpp26/basic.contract.eval.p17-SMF2.C | 2 +-
.../cpp26/basic.contract.eval.p7.3.C | 1 +
.../contracts/cpp26/callerside-checks/ctor.C | 2 +-
.../g++.dg/contracts/cpp26/non-const1.C | 20 +++
7 files changed, 115 insertions(+), 44 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/contracts/cpp26/non-const1.C
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 3209d435029..3c92281cdd0 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -53,6 +53,7 @@ do {
\
static HOST_WIDE_INT find_array_ctor_elt (tree ary, tree dindex,
bool insert = false);
static int array_index_cmp (tree key, tree index);
+static bool check_for_failed_contracts (const constexpr_ctx *ctx);
/* Returns true iff FUN is an instantiation of a constexpr function
template or a defaulted constexpr function. */
@@ -11260,42 +11261,43 @@ cxx_eval_constant_expression (const constexpr_ctx
*ctx, tree t,
r = void_node;
/* Only record the first fail, and do not go further is the semantic
is 'ignore'. */
- if (*non_constant_p || ctx->global->contract_statement
+ if (*non_constant_p
+ || (ctx->global->contract_statement
+ && contract_terminating_p (ctx->global->contract_statement))
|| contract_ignored_p (t))
break;
tree cond = CONTRACT_CONDITION (t);
- if (!potential_rvalue_constant_expression (cond))
+ bool ctrct_non_const_p = !potential_rvalue_constant_expression (cond);
+ tree eval = boolean_true_node;
+ if (!ctrct_non_const_p)
{
- ctx->global->contract_statement = t;
- ctx->global->contract_condition_non_const = true;
- break;
+ /* We need to evaluate and stash the result of this here, since
+ whether it needs to be reported (and how) depends on whether the
+ containing expression is otherwise const. */
+ bool ctrct_overflow_p = false;
+ tree jmp_target = NULL_TREE;
+ constexpr_ctx new_ctx = *ctx;
+ new_ctx.quiet = true;
+ /* Avoid modification of existing values. */
+ modifiable_tracker ms (new_ctx.global);
+ eval = (cxx_eval_constant_expression
+ (&new_ctx, cond, vc_prvalue, &ctrct_non_const_p,
+ &ctrct_overflow_p, &jmp_target));
}
- /* We need to evaluate and stash the result of this here, since whether
- it needs to be reported (and how) depends on whether the containing
- expression is otherwise const. */
- bool ctrct_non_const_p = false;
- bool ctrct_overflow_p = false;
- tree jmp_target = NULL_TREE;
- constexpr_ctx new_ctx = *ctx;
- new_ctx.quiet = true;
- /* Avoid modification of existing values. */
- modifiable_tracker ms (new_ctx.global);
- tree eval =
- cxx_eval_constant_expression (&new_ctx, cond, vc_prvalue,
- &ctrct_non_const_p,
- &ctrct_overflow_p, &jmp_target);
- /* Not a constant. */
- if (ctrct_non_const_p)
+ /* Not a constant or check failed. */
+ if (ctrct_non_const_p || integer_zerop (eval))
{
ctx->global->contract_statement = t;
- ctx->global->contract_condition_non_const = true;
- break;
+ ctx->global->contract_condition_non_const = ctrct_non_const_p;
}
- /* Constant, but check failed. */
- if (integer_zerop (eval))
- ctx->global->contract_statement = t;
+
+ /* If diagnosing, do it while we have the cx_call_context. */
+ if (!ctx->quiet
+ && check_for_failed_contracts (ctx))
+ /* And for a terminating semantic, stop evaluating. */
+ *non_constant_p = true;
}
break;
@@ -11521,13 +11523,15 @@ mark_non_constant (tree t)
a failed or non-constant contract that would invalidate this. */
static bool
-check_for_failed_contracts (constexpr_ctx *ctx)
+check_for_failed_contracts (const constexpr_ctx *ctx)
{
constexpr_global_ctx *const global_ctx = ctx->global;
if (!flag_contracts || !global_ctx->contract_statement)
return false;
location_t loc = EXPR_LOCATION (global_ctx->contract_statement);
+ tree cond = CONTRACT_CONDITION (global_ctx->contract_statement);
+ loc = cp_expr_loc_or_loc (cond, loc);
enum diagnostics::kind kind;
bool error = false;
/* [intro.compliance.general]/2.3.4. */
@@ -11545,16 +11549,24 @@ check_for_failed_contracts (constexpr_ctx *ctx)
else
kind = diagnostics::kind::warning;
+ auto_diagnostic_group d;
+
/* [basic.contract.eval]/7.3 */
if (global_ctx->contract_condition_non_const)
{
emit_diagnostic (kind, loc, 0, "contract condition is not constant");
+ if (error)
+ cxx_constant_value (cond);
return error;
}
/* Otherwise, the evaluation was const, but determined to be false. */
emit_diagnostic (kind, loc, 0,
"contract predicate is false in constant expression");
+ /* Workaround: Avoid redundant "in expansion of macro". */
+ loc = linemap_resolve_location (line_table, loc,
+ LRK_SPELLING_LOCATION, NULL);
+ diagnose_failing_condition (cond, loc, /*show_expr*/true, ctx);
return error;
}
@@ -11873,6 +11885,57 @@ cxx_eval_outermost_constant_expr (tree t, bool
allow_non_constant,
if (!non_constant_p && overflow_p)
non_constant_p = true;
+ /* If allow_non_constant, we delay diagnosing contracts until we know
+ that the expression is otherwise constant.
+
+ Various constexpr contract violation scenarios:
+ * MCE, non-SFINAE, terminating: diagnose and mark non-constant
+ during evaluation
+ * MCE, non-SFINAE, non-terminating: diagnose during evaluation
+ * MCE, SFINAE, terminating: diagnose in repeated eval, return
+ error_mark_node for SFINAE
+ * MCE, SFINAE, non-terminating: diagnose in repeated eval
+ * non-MCE: diagnose in repeated eval, set non-constant so run-time
+ violation handler also sees the violation
+
+ For MCE+terminating, the necessary semantics are just a hard error; what
+ happens after that is error-recovery.
+
+ In the non-SFINAE (!allow_non_constant) case I think it's clearly
+ preferable to stop evaluation after the violation, which we do by marking
+ the evaluation non-constant. We could work harder to return
+ error_mark_node instead like the SFINAE case below, but that seems
+ unnecessary.
+
+ The allow_non_constant case is trickier, because that expresses both
+ actual SFINAE and cases like finish_static_assert where we do one trial
+ evaluation to see if it's constant, and then another to get the actual
+ error if needed. So if we set non_constant_p like for the
+ !allow_non_constant case, we get SFINAE consistent with
+ e.g. _GLIBCXX_ASSERT_FAIL, but we get redundant errors with static_assert.
+ We can finesse it a bit more by returning error_mark_node, which
+ finish_static_assert distinguishes from non-constant.
+
+ ??? cp_fold_immediate_r should change to follow the finish_static_assert
+ pattern (using maybe_constant_value instead of cxx_constant_value for the
+ trial evaluation) once the consteval-only values overhaul goes in. */
+
+ if (!non_constant_p && allow_non_constant
+ && global_ctx.contract_statement)
+ {
+ /* Repeat evaluation to get cx_call_context for the diagnostic. */
+ cxx_eval_outermost_constant_expr (t, /*allow*/false, strict,
+ manifestly_const_eval,
+ constexpr_dtor, object);
+ if (ctx.manifestly_const_eval != mce_true)
+ /* Treat it as non-constant so the violation is also detectable at
+ runtime (if the compile succeeds because we're observing). */
+ non_constant_p = true;
+ else if (contract_terminating_p (global_ctx.contract_statement))
+ /* Substitution failure (see discussion above). */
+ return error_mark_node;
+ }
+
/* Unshare the result. */
bool should_unshare = true;
if (r == t || (TREE_CODE (t) == TARGET_EXPR
@@ -11881,19 +11944,6 @@ cxx_eval_outermost_constant_expr (tree t, bool
allow_non_constant,
if (non_constant_p && !allow_non_constant)
return error_mark_node;
- else if (check_for_failed_contracts (&ctx))
- {
- if (manifestly_const_eval == mce_true)
- /* If MCE, we gave a hard error and return error_mark_node. */
- return error_mark_node;
- else
- {
- /* Otherwise treat it as non-constant so the violation is still
- detectable at run-time. */
- gcc_checking_assert (allow_non_constant);
- return t;
- }
- }
else if (non_constant_p && TREE_CONSTANT (r))
r = mark_non_constant (r);
else if (non_constant_p)
diff --git
a/gcc/testsuite/g++.dg/contracts/cpp26/basic.contract.eval.p17-SMF-post.C
b/gcc/testsuite/g++.dg/contracts/cpp26/basic.contract.eval.p17-SMF-post.C
index 2aaf13781f8..2010809618a 100644
--- a/gcc/testsuite/g++.dg/contracts/cpp26/basic.contract.eval.p17-SMF-post.C
+++ b/gcc/testsuite/g++.dg/contracts/cpp26/basic.contract.eval.p17-SMF-post.C
@@ -35,7 +35,7 @@ void handle_contract_violation(const
std::contracts::contract_violation& violati
struct X
{
- X(const int x) noexcept post(x>1) try
+ X(const int x) noexcept post(x>1) try // { dg-warning "false" "" { target
implicit_constexpr } }
{
int i = 1;
}
diff --git
a/gcc/testsuite/g++.dg/contracts/cpp26/basic.contract.eval.p17-SMF-pre.C
b/gcc/testsuite/g++.dg/contracts/cpp26/basic.contract.eval.p17-SMF-pre.C
index 378b93d2bbc..ef582236744 100644
--- a/gcc/testsuite/g++.dg/contracts/cpp26/basic.contract.eval.p17-SMF-pre.C
+++ b/gcc/testsuite/g++.dg/contracts/cpp26/basic.contract.eval.p17-SMF-pre.C
@@ -35,7 +35,7 @@ void handle_contract_violation(const
std::contracts::contract_violation& violati
struct X
{
- X(int x) noexcept pre(x>1) try
+ X(int x) noexcept pre(x>1) try // { dg-warning "false" "" { target
implicit_constexpr } }
{
int i = 1;
}
diff --git
a/gcc/testsuite/g++.dg/contracts/cpp26/basic.contract.eval.p17-SMF2.C
b/gcc/testsuite/g++.dg/contracts/cpp26/basic.contract.eval.p17-SMF2.C
index eccef309f4b..9de275c04f3 100644
--- a/gcc/testsuite/g++.dg/contracts/cpp26/basic.contract.eval.p17-SMF2.C
+++ b/gcc/testsuite/g++.dg/contracts/cpp26/basic.contract.eval.p17-SMF2.C
@@ -34,7 +34,7 @@ void handle_contract_violation(const
std::contracts::contract_violation& violati
struct X
{
- X(int x) noexcept pre(x>1)
+ X(int x) noexcept pre(x>1) // { dg-warning "false" "" { target
implicit_constexpr } }
{
try{
int i = 1;
diff --git a/gcc/testsuite/g++.dg/contracts/cpp26/basic.contract.eval.p7.3.C
b/gcc/testsuite/g++.dg/contracts/cpp26/basic.contract.eval.p7.3.C
index 6371c586dba..ea322cc0bf2 100644
--- a/gcc/testsuite/g++.dg/contracts/cpp26/basic.contract.eval.p7.3.C
+++ b/gcc/testsuite/g++.dg/contracts/cpp26/basic.contract.eval.p7.3.C
@@ -9,6 +9,7 @@ extern bool runtime_check ();
consteval void foo( auto x ) pre ( runtime_check () ) {}
// { dg-error {contract condition is not constant} "" { target *-*-* } .-1 }
+// { dg-message {runtime_check} "" { target *-*-* } .-2 }
int main() {
foo( 42 ); // { dg-error {call to consteval function 'foo<int>\(42\)' is not
a constant expression} }
}
diff --git a/gcc/testsuite/g++.dg/contracts/cpp26/callerside-checks/ctor.C
b/gcc/testsuite/g++.dg/contracts/cpp26/callerside-checks/ctor.C
index 3aff0276601..b2d3a6062cc 100644
--- a/gcc/testsuite/g++.dg/contracts/cpp26/callerside-checks/ctor.C
+++ b/gcc/testsuite/g++.dg/contracts/cpp26/callerside-checks/ctor.C
@@ -4,7 +4,7 @@
struct X
{
X (int x) noexcept
- pre (x>1)
+ pre (x>1) // { dg-error "false" "" { target
implicit_constexpr } }
{
try {
int i = 1;
diff --git a/gcc/testsuite/g++.dg/contracts/cpp26/non-const1.C
b/gcc/testsuite/g++.dg/contracts/cpp26/non-const1.C
new file mode 100644
index 00000000000..7aa453227bd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/contracts/cpp26/non-const1.C
@@ -0,0 +1,20 @@
+// { dg-additional-options -fcontracts }
+
+struct A {
+ const char *p;
+ unsigned len;
+
+ constexpr const char&
+ operator[] (unsigned i) const
+ {
+ contract_assert (i < len);
+ return p[i];
+ }
+};
+
+void f(A a)
+{
+ /* This shouldn't be an error just because the contract predicate is
+ non-constant -- so is the trial evaluation. */
+ const char &c = a[0];
+}
base-commit: 8a2d0ea99609812c896a098b2f93298b0ca7e965
--
2.56.0
