clang evaluates is_constant_evaluated incorrectly with narrowing conversion

[efriedma-quic]

Consider the following example:

#include <type_traits>
struct X { unsigned u; };
void f(X x);
void g(int a) {
    f({0xFFFFFFFFLL + std::is_constant_evaluated()});
}

The standard says the conversion is narrowing if "the source is a constant expression whose value after integral promotions will fit into the target type". gcc correctly rejects because of this: it treats the expression as a constant expression, then it fails to narrow. clang incorrectly accepts because isCXX11ConstantExpr() doesn't treat the evaluation as a constant context.

We have to be a little careful here... consider the following variant:

#include <type_traits>
struct X { unsigned u; };
void f(X x);
void g(int a) {
    f({0x100000000LL - std::is_constant_evaluated()});
}

clang incorrectly rejects. gcc miscompiles: it checks the narrowing in a constant context, but then doesn't evaluate the actual expression in a constant context, so the integer overflows to 0.

(I don't expect anyone to intentionally do this... wrote some synthetic testcases while looking at isCXX11ConstantExpr() for other reasons.)

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: Eli Friedman (efriedma-quic)

Consider the following example:

#include <type_traits>
struct X { unsigned u; };
void f(X x);
void g(int a) {
    f({0xFFFFFFFFLL + std::is_constant_evaluated()});
}

The standard says the conversion is narrowing if "the source is a constant expression whose value after integral promotions will fit into the target type". gcc correctly rejects because of this: it treats the expression as a constant expression, then it fails to narrow. clang incorrectly accepts because isCXX11ConstantExpr() doesn't treat the evaluation as a constant context.

We have to be a little careful here... consider the following variant:

#include <type_traits>
struct X { unsigned u; };
void f(X x);
void g(int a) {
    f({0x100000000LL - std::is_constant_evaluated()});
}

clang incorrectly rejects. gcc miscompiles: it checks the narrowing in a constant context, but then doesn't evaluate the actual expression in a constant context, so the integer overflows to 0.

(I don't expect anyone to intentionally do this... wrote some synthetic testcases while looking at isCXX11ConstantExpr() for other reasons.)

[zygoloid]

std::is_constant_evaluated() only evaluates to true in manifestly constant-evaluated contexts. See:

• https://eel.is/c++draft/meta.const.eval (is_constant_evaluated is equivalent to if consteval)
• https://eel.is/c++draft/stmt.if#5 (if consteval behavior depends on whether the statement is in a manifestly constant-evaluated context)
• https://eel.is/c++draft/expr.const#28 (narrowing conversion checks are not manifestly constant-evaluated)

It looks to me like this is a GCC bug, not a Clang bug.

[efriedma-quic]

Does "the source is a constant expression" in [dcl.init.list]p7 mean something different from a "constant-expression"? Is it supposed to be "the source is a core constant expression", or something like that?

[zygoloid]

The italic term constant-expression refers to that specific grammar production, and only includes the places where that term is used in the grammar. This includes things that are syntactically required to be constants, like case label values, static_assert conditions, array bounds (other than the outermost bound of a new type), enumerator values, widths of bit-fields, and template arguments. The upright term "constant expression" is the term defined in [expr.const]/22, and applies to any expression that can be evaluated to a value that's reasonable to escape from constant evaluation, using the standard evaluation rules. ("Core constant expression" is the same but without the check that the resulting value is reasonable. "Reasonable" here means that it's fully-initialized and doesn't escape anything that's consteval. We also used to disallow escaping automatic storage duration objects but it looks like that restriction got relaxed at some point.)

Overall, the intention is that std::is_constant_evaluated only returns true in the cases where the expression is definitely going to be constant evaluated, not in cases where it's being evaluated for diagnostics or as an optimization or something like that. So it's intended to evaluate to false in cases like the narrowing check.

[efriedma-quic]

Given that, I guess clang correctly accepts the following?

  struct X { unsigned u; };
  constexpr int f(X x) {return x.u;}
  static_assert(f({0xFFFFFFFFLL + __builtin_is_constant_evaluated()}) == 0);

It's counterintuitive that the rule against narrowing conversions doesn't actually prevent narrowing conversions, but I guess it's self-consistent.

I'll add some tests to the clang regression tests.

[frederick-vs-ja]

Given that, I guess clang correctly accepts the following?

  struct X { unsigned u; };
  constexpr int f(X x) {return x.u;}
  static_assert(f({0xFFFFFFFFLL + __builtin_is_constant_evaluated()}) == 0);

It's counterintuitive that the rule against narrowing conversions doesn't actually prevent narrowing conversions, but I guess it's self-consistent.

I'll add some tests to the clang regression tests.

It seems to me that this should be rejected per [dcl.init.list]/7.4.2.

[zygoloid]

It's not incredibly clear to me what https://eel.is/c++draft/stmt.if#5 means when it says "evaluated in a context that is manifestly constant-evaluated", but I think it's referring to whether the expression E in [expr.const]/10 is manifestly constant-evaluated. So I think we're doing the right thing in the static_assert case too.

[frederick-vs-ja]

It's not incredibly clear to me what https://eel.is/c++draft/stmt.if#5 means when it says "evaluated in a context that is manifestly constant-evaluated", but I think it's referring to whether the expression E in [expr.const]/10 is manifestly constant-evaluated. So I think we're doing the right thing in the static_assert case too.

I agree on that __builtin_is_constant_evaluated() should return true here. But given when __builtin_is_constant_evaluated() is replaced with a plain true, Clang (IMO correctly) rejects the example (https://godbolt.org/z/z56ve6Kaz), I think it's a bug to accept the original case using __builtin_is_constant_evaluated().

Edit: #119928 is possibly related.

[efriedma-quic]

The interpretation we're going with is that the list of "manifestly constant-evaluated" contexts should be interpreted narrowly: it only applies when we're evaluating the expression as a whole. If we're evaluating a sub-expression for some other reason (narrowing conversions, the operand of "new", etc.), it's not manifestly constant-evaluated, even if it's a sub-expression of a manifestly constant-evaluated expression.

So when we check narrowing, __builtin_is_constant_evaluated() is false, but when we evaluate the static_assert(), __builtin_is_constant_evaluated() is true. Which is a little weird, but we can't actually tell if the parent expression is a manifestly constant-evaluated context when we do overload resolution. So the only alternative is treating narrowing conversions as always manifestly constant-evaluated.

We could give an error if a conversion that we're treating as non-narrowing actually truncates bits during constant evaluation. But that doesn't really seem worth it.