rsmith added a comment. In D55500#1347493 <https://reviews.llvm.org/D55500#1347493>, @EricWF wrote:
> @rsmith, what sorts of additional tests are needed? Let's see... - test that we use the constant initializer for suitably-constant-initialized locals even if we can't constant-fold the reference to them: bool f(int k) { const bool n = is_constant_evaluated(); const bool *volatile p = n; return *p; } - test that we suitably handle local references initialized by constant expressions (and local `constexpr` variables), and non-`const` globals - test that `__builtin_is_constant_evaluated()` returns false in contexts that IR generation might constant-fold (eg, the operand of a non-`constexpr` `if` or the left-hand side of a `?:` expression) - ensure that the right value is returned in all the cases that are syntactically //constant-expression//s in the C++ grammar: - second or subsequent array bound in a //new-expression// - `case` labels - operand of `static_assert` - array bounds (but be careful around the treatment of VLAs! for them we want the "try evaluating it as a constant expression and fall back to treating it as a non-constant expression" rule that we have for the initializers of globals) - values of enumerators - operand of `alignas` - width of a bit-field - template arguments (but bizarrely not default template arguments! I think that's a bug and I'll bring it up on the core reflector.) - operand of `noexcept` (and eventually `explicit` -- please write a test for the latter with a FIXME to update the expectations, so we don't forget) ================ Comment at: include/clang/Basic/Builtins.def:758 +// Random C++ builtins. +LANGBUILTIN(__builtin_is_constant_evaluated, "b", "ncu", CXX_LANG) ---------------- This should be grouped with `__builtin_launder`, though maybe moving that one here makes more sense than grouping them both under "GCC builtins" since they're directly implementing C++ stdlib functions. ================ Comment at: include/clang/Basic/Builtins.def:759 +// Random C++ builtins. +LANGBUILTIN(__builtin_is_constant_evaluated, "b", "ncu", CXX_LANG) + ---------------- bruno wrote: > EricWF wrote: > > EricWF wrote: > > > bruno wrote: > > > > Name bikeshedding : perhaps the builtin name could be detached from the > > > > std:: name? Suggestion: `__builtin_in_constant_evaluation_context` > > > I'm not sure detaching it from the `std::` name is desirable. Most > > > importantly it should match w/e GCC does/decides to do. > > > > > > But if it is, we should name in deference to the standardese it > > > implements. Specifically weither an expression or conversion is > > > //manifestly constant-evaluated// > > > [[expr.const](http://eel.is/c++draft/expr.const#11)]p11. > > > > > > Therefore I proffer `__builtin_is_manifestly_constant_evaluated()` or > > > `__builtin_is_being_manifestly_constant_evaluated()`. > > > > > > > > Actually, GCC has `__builtin_is_constant_evaluated` so we should use that > > name too. > Agreed! I don't think the "u" flag has any meaning here (there are no arguments, so no unevaluated arguments), and it would reduce the complexity of this declaration if you remove it. I don't think the "c" flag is formally correct -- a program can contain two calls to `__builtin_is_constant_evaluated()` that return different values (even "U" appears to be incorrect). It would, for example, be reasonable for Clang to warn on: ``` const bool b = pure_fn(); return b == pure_fn(); ``` ... on the basis that the comparison must always evaluate to `true`. But such a warning would be incorrect for `__builtin_is_constant_evaluated`, so it's not a pure function. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55500/new/ https://reviews.llvm.org/D55500 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits