[Diagnostics] Warn for std::is_constant_evaluated in constexpr mode

[xbolva00]

Summary

constexpr int fn1() {

if constexpr (std::is_constant_evaluated()) // condition is always true!
  return 0;
else
  return 1;

}

constexpr int fn2() {

if (std::is_constant_evaluated())
  return 0;
else
  return 1;

}

Solves PR42977

[xbolva00]

clang/lib/Sema/SemaExprCXX.cpp

3679 ↗

(On Diff #226702)

Not ideal place. Missing !E case, etc...

Maybe in ActOnCompletedExpr?

[xbolva00]

Support !E.

[xbolva00]

New tests

[rsmith]

clang/include/clang/Basic/DiagnosticSemaKinds.td

8410 ↗

(On Diff #226759)

"constexpr mode" isn't a thing. Maybe just "in this context"?

I think this should have its own warning group, not be grouped under -Wtautological-compare.

clang/lib/Sema/SemaExprCXX.cpp

3694 ↗

(On Diff #226759)

As noted in PR42977, we should warn whenever we find a call to is_constant_evaluated in a manifestly constant evaluated expression, not only in the condition of a constexpr if.

This can be checked in ExprConstant.cpp when we evaluate a call to __builtin_is_constant_evaluated: if we're about to return true from __builtin_is_constant_evaluated, and there's either no calls on the call stack or exactly one call which is to std::is_constant_evaluated, then warn.

clang/test/SemaCXX/warn-std-is-constant-evaluated-constexpr.cpp

2 ↗

(On Diff #226759)

This second RUN line doesn't add any value; remove it?

[xbolva00]

clang/include/clang/Basic/DiagnosticSemaKinds.td

8410 ↗

(On Diff #226759)

I agree. just... gcc put it under -Wtautological-compare. So should I create a new subgroup of -Wtautological-compare? Or just a new group?

clang/lib/Sema/SemaExprCXX.cpp

3694 ↗

(On Diff #226759)

Yeah, thanks for the hint!

clang/test/SemaCXX/warn-std-is-constant-evaluated-constexpr.cpp

2 ↗

(On Diff #226759)

Right, I will remove it.

[rsmith]

clang/include/clang/Basic/DiagnosticSemaKinds.td

8410 ↗

(On Diff #226759)

I don't really imagine many people wanting to turn this warning (or -Wtautological-compare) off, so I don't think it matters all that much. But making this a subgroup of -Wtautological-compare is cheap and would preserve GCC compatibility, so let's do that.

[xbolva00]

• Merge branch 'master' of https://github.com/llvm/llvm-project

[xbolva00]

Removed old unused change.

[xbolva00]

clang/lib/AST/ExprConstant.cpp

10598

!Info.CheckingPotentialConstantExpression

Basically just to silence test case:
namespace std {
constexpr bool is_constant_evaluated() noexcept {

return __builtin_is_constant_evaluated();

}
}

In real world code, this will not warn, since call's loc is in system header. Leave it?

10603

Without this condition, it warns 3 times for

if constexpr (std::is_constant_evaluated() == false)

[rsmith]

clang/include/clang/Basic/DiagnosticASTKinds.td

333

"constexpr context" doesn't mean anything, and I'm not sure it has the right implications (eg, is the body of a constexpr function a constexpr context?).

The technically-correct thing would be "in a manifestly constant-evaluated expression".

334

Looks like you decided not to make this a subgroup of tautological-compare? (I'm OK with that, I just wanted to make sure this was intentional given the prior discussion.)

clang/lib/AST/ExprConstant.cpp

10598

The check seems right to me.

10600

The number of arguments check is redundant; we know the builtin has no parameters.

10603

Please add a "// FIXME: Find a better way to avoid duplicated diagnostics." or similar. This is relying somewhat on the implementation details of callers of the constant evaluator.

[xbolva00]

clang/include/clang/Basic/DiagnosticASTKinds.td

334

Yes, I decided to go with a new group since the warning is more powerful than gcc's version of this warning.

The technically-correct thing would be "in a manifestly constant-evaluated expression".

I will use this message - Thanks!

[xbolva00]

Fixed review comments.

[xbolva00]

Thanks you for the review and advices!

[davezarzycki]

This broke two stage builds of libc++. Can we revert this until a fix is ready?

FAIL: libc++ :: std/utilities/meta/meta.const.eval/is_constant_evaluated.fail.cpp (55845 of 59103)
******************** TEST 'libc++ :: std/utilities/meta/meta.const.eval/is_constant_evaluated.fail.cpp' FAILED ********************
Command: ['/p/tllvm/bin/clang++', '-o', '/dev/null', '-x', 'c++', '/home/dave/s/lp/libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.fail.cpp', '-c', '-v', '-ftemplate-depth=270', '-fsyntax-only', '-Xclang', '-verify', '-Xclang', '-verify-ignore-unexpected=note', '-ferror-limit=1024', '-Werror=thread-safety', '-std=c++2a', '-include', '/home/dave/s/lp/libcxx/test/support/nasty_macros.h', '-nostdinc++', '-I/home/dave/s/lp/libcxx/include', '-I/tmp/_update_lc/t/projects/libcxx/include/c++build', '-D__STDC_FORMAT_MACROS', '-D__STDC_LIMIT_MACROS', '-D__STDC_CONSTANT_MACROS', '-I/home/dave/s/lp/libcxx/test/support', '-DLIBCXX_FILESYSTEM_STATIC_TEST_ROOT="/home/dave/s/lp/libcxx/test/std/input.output/filesystems/Inputs/static_test_env"', '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT="/tmp/_update_lc/t/projects/libcxx/test/filesystem/Output/dynamic_env"', '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER="/usr/bin/python /home/dave/s/lp/libcxx/test/support/filesystem_dynamic_test_helper.py"', '-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER', '-Wall', '-Wextra', '-Werror', '-Wuser-defined-warnings', '-Wshadow', '-Wno-unused-command-line-argument', '-Wno-attributes', '-Wno-pessimizing-move', '-Wno-c++11-extensions', '-Wno-user-defined-literals', '-Wno-noexcept-type', '-Wsign-compare', '-Wunused-variable', '-Wunused-parameter', '-Wunreachable-code', '-Wno-error=user-defined-warnings', '-c']
Exit Code: 1
Standard Error:
--
clang version 10.0.0 (/home/dave/s/lp/clang 7e1a3076419d4d453d71143a1e81409ea1db177c)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /p/tllvm/bin
Found candidate GCC installation: /usr/lib/gcc/x86_64-redhat-linux/9
Selected GCC installation: /usr/lib/gcc/x86_64-redhat-linux/9
Candidate multilib: .;@m64
Candidate multilib: 32;@m32
Selected multilib: .;@m64
 "/p/tllvm/bin/clang-10" -cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -disable-free -disable-llvm-verifier -discard-value-names -main-file-name is_constant_evaluated.fail.cpp -mrelocation-model static -mthread-model posix -mframe-pointer=all -fmath-errno -masm-verbose -mconstructor-aliases -munwind-tables -fuse-init-array -target-cpu x86-64 -dwarf-column-info -debugger-tuning=gdb -v -nostdinc++ -resource-dir /p/tllvm/lib64/clang/10.0.0 -include /home/dave/s/lp/libcxx/test/support/nasty_macros.h -I /home/dave/s/lp/libcxx/include -I /tmp/_update_lc/t/projects/libcxx/include/c++build -D __STDC_FORMAT_MACROS -D __STDC_LIMIT_MACROS -D __STDC_CONSTANT_MACROS -I /home/dave/s/lp/libcxx/test/support -D "LIBCXX_FILESYSTEM_STATIC_TEST_ROOT=\"/home/dave/s/lp/libcxx/test/std/input.output/filesystems/Inputs/static_test_env\"" -D "LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT=\"/tmp/_update_lc/t/projects/libcxx/test/filesystem/Output/dynamic_env\"" -D "LIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER=\"/usr/bin/python /home/dave/s/lp/libcxx/test/support/filesystem_dynamic_test_helper.py\"" -D _LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -internal-isystem /usr/local/include -internal-isystem /p/tllvm/lib64/clang/10.0.0/include -internal-externc-isystem /include -internal-externc-isystem /usr/include -Werror=thread-safety -Wall -Wextra -Werror -Wuser-defined-warnings -Wshadow -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-user-defined-literals -Wno-noexcept-type -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-error=user-defined-warnings -std=c++2a -fdeprecated-macro -fdebug-compilation-dir /tmp/_update_lc/t -ftemplate-depth 270 -ferror-limit 1024 -fmessage-length 0 -fgnuc-version=4.2.1 -fno-implicit-modules -fobjc-runtime=gcc -fcxx-exceptions -fexceptions -fdiagnostics-show-option -verify -verify-ignore-unexpected=note -faddrsig -x c++ /home/dave/s/lp/libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.fail.cpp
clang -cc1 version 10.0.0 based upon LLVM 10.0.0svn default target x86_64-unknown-linux-gnu
ignoring nonexistent directory "/include"
#include "..." search starts here:
#include <...> search starts here:
 /home/dave/s/lp/libcxx/include
 /tmp/_update_lc/t/projects/libcxx/include/c++build
 /home/dave/s/lp/libcxx/test/support
 /usr/local/include
 /p/tllvm/lib64/clang/10.0.0/include
 /usr/include
End of search list.
error: 'error' diagnostics seen but not expected: 
  File /home/dave/s/lp/libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.fail.cpp Line 26: 'std::is_constant_evaluated' will always evaluate to 'true' in a manifestly constant-evaluated expression
1 error generated.
--

Expected compilation using verify to pass!

********************
FAIL: libc++ :: std/utilities/meta/meta.const.eval/is_constant_evaluated.pass.cpp (55846 of 59103)
******************** TEST 'libc++ :: std/utilities/meta/meta.const.eval/is_constant_evaluated.pass.cpp' FAILED ********************
Command: ['/p/tllvm/bin/clang++', '-o', '/tmp/_update_lc/t/projects/libcxx/test/std/utilities/meta/meta.const.eval/Output/is_constant_evaluated.pass.cpp.o', '-x', 'c++', '/home/dave/s/lp/libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.pass.cpp', '-c', '-v', '-ftemplate-depth=270', '-Werror=thread-safety', '-std=c++2a', '-include', '/home/dave/s/lp/libcxx/test/support/nasty_macros.h', '-nostdinc++', '-I/home/dave/s/lp/libcxx/include', '-I/tmp/_update_lc/t/projects/libcxx/include/c++build', '-D__STDC_FORMAT_MACROS', '-D__STDC_LIMIT_MACROS', '-D__STDC_CONSTANT_MACROS', '-I/home/dave/s/lp/libcxx/test/support', '-DLIBCXX_FILESYSTEM_STATIC_TEST_ROOT="/home/dave/s/lp/libcxx/test/std/input.output/filesystems/Inputs/static_test_env"', '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT="/tmp/_update_lc/t/projects/libcxx/test/filesystem/Output/dynamic_env"', '-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER="/usr/bin/python /home/dave/s/lp/libcxx/test/support/filesystem_dynamic_test_helper.py"', '-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER', '-Wall', '-Wextra', '-Werror', '-Wuser-defined-warnings', '-Wshadow', '-Wno-unused-command-line-argument', '-Wno-attributes', '-Wno-pessimizing-move', '-Wno-c++11-extensions', '-Wno-user-defined-literals', '-Wno-noexcept-type', '-Wsign-compare', '-Wunused-variable', '-Wunused-parameter', '-Wunreachable-code', '-c']
Exit Code: 1
Standard Error:
--
clang version 10.0.0 (/home/dave/s/lp/clang 7e1a3076419d4d453d71143a1e81409ea1db177c)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /p/tllvm/bin
Found candidate GCC installation: /usr/lib/gcc/x86_64-redhat-linux/9
Selected GCC installation: /usr/lib/gcc/x86_64-redhat-linux/9
Candidate multilib: .;@m64
Candidate multilib: 32;@m32
Selected multilib: .;@m64
 "/p/tllvm/bin/clang-10" -cc1 -triple x86_64-unknown-linux-gnu -emit-obj -mrelax-all -disable-free -disable-llvm-verifier -discard-value-names -main-file-name is_constant_evaluated.pass.cpp -mrelocation-model static -mthread-model posix -mframe-pointer=all -fmath-errno -masm-verbose -mconstructor-aliases -munwind-tables -fuse-init-array -target-cpu x86-64 -dwarf-column-info -debugger-tuning=gdb -v -nostdinc++ -resource-dir /p/tllvm/lib64/clang/10.0.0 -include /home/dave/s/lp/libcxx/test/support/nasty_macros.h -I /home/dave/s/lp/libcxx/include -I /tmp/_update_lc/t/projects/libcxx/include/c++build -D __STDC_FORMAT_MACROS -D __STDC_LIMIT_MACROS -D __STDC_CONSTANT_MACROS -I /home/dave/s/lp/libcxx/test/support -D "LIBCXX_FILESYSTEM_STATIC_TEST_ROOT=\"/home/dave/s/lp/libcxx/test/std/input.output/filesystems/Inputs/static_test_env\"" -D "LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT=\"/tmp/_update_lc/t/projects/libcxx/test/filesystem/Output/dynamic_env\"" -D "LIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER=\"/usr/bin/python /home/dave/s/lp/libcxx/test/support/filesystem_dynamic_test_helper.py\"" -D _LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -internal-isystem /usr/local/include -internal-isystem /p/tllvm/lib64/clang/10.0.0/include -internal-externc-isystem /include -internal-externc-isystem /usr/include -Werror=thread-safety -Wall -Wextra -Werror -Wuser-defined-warnings -Wshadow -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-user-defined-literals -Wno-noexcept-type -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -std=c++2a -fdeprecated-macro -fdebug-compilation-dir /tmp/_update_lc/t/projects/libcxx/test/std/utilities/meta/meta.const.eval -ftemplate-depth 270 -ferror-limit 19 -fmessage-length 0 -fgnuc-version=4.2.1 -fno-implicit-modules -fobjc-runtime=gcc -fcxx-exceptions -fexceptions -fdiagnostics-show-option -faddrsig -o /tmp/_update_lc/t/projects/libcxx/test/std/utilities/meta/meta.const.eval/Output/is_constant_evaluated.pass.cpp.o -x c++ /home/dave/s/lp/libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.pass.cpp
clang -cc1 version 10.0.0 based upon LLVM 10.0.0svn default target x86_64-unknown-linux-gnu
ignoring nonexistent directory "/include"
#include "..." search starts here:
#include <...> search starts here:
 /home/dave/s/lp/libcxx/include
 /tmp/_update_lc/t/projects/libcxx/include/c++build
 /home/dave/s/lp/libcxx/test/support
 /usr/local/include
 /p/tllvm/lib64/clang/10.0.0/include
 /usr/include
End of search list.
/home/dave/s/lp/libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.pass.cpp:35:24: error: 'std::is_constant_evaluated' will always evaluate to 'true' in a manifestly constant-evaluated expression [-Werror,-Wconstant-evaluated]
    constexpr bool p = std::is_constant_evaluated();
                       ^
/home/dave/s/lp/libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.pass.cpp:41:19: error: 'std::is_constant_evaluated' will always evaluate to 'true' in a manifestly constant-evaluated expression [-Werror,-Wconstant-evaluated]
    static_assert(std::is_constant_evaluated(), "");
                  ^
/home/dave/s/lp/libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.pass.cpp:44:33: error: 'std::is_constant_evaluated' will always evaluate to 'true' in a manifestly constant-evaluated expression [-Werror,-Wconstant-evaluated]
    ASSERT_SAME_TYPE(InTemplate<std::is_constant_evaluated()>, InTemplate<true>);
                                ^
3 errors generated.
--

Compilation failed unexpectedly!
********************
Testing Time: 111.41s
********************
Failing Tests (2):
    libc++ :: std/utilities/meta/meta.const.eval/is_constant_evaluated.fail.cpp
    libc++ :: std/utilities/meta/meta.const.eval/is_constant_evaluated.pass.cpp

  Expected Passes    : 43344
  Expected Failures  : 103
  Unsupported Tests  : 15654
  Unexpected Failures: 2

[xbolva00]

Line 918

Add:
self.cxx.addWarningFlagIfSupported('-Wno-constant-evaluated')

to libcxx/utils/libcxx/test/config.py

Can you please try?

cc @EricWF

[davezarzycki]

Hi @xbolva00 – Thanks. That does make the libcxx test suite pass.

[xbolva00]

Thanks! Commited in rGdba83965722b540f6baf43163210943c41e1378a

clang/include/clang/Basic/DiagnosticASTKinds.td

def warn_integer_constant_overflow : Warning<
   InGroup<DiagGroup<"integer-overflow">>;
 
 // This is a temporary diagnostic, and shall be removed once our
 // implementation is complete, and like the preceding constexpr notes belongs
 // in Sema.
 def note_unimplemented_constexpr_lambda_feature_ast : Note<
     "unimplemented constexpr lambda feature: %0 (coming soon!)">;
 
+def warn_is_constant_evaluated_always_true_constexpr : Warning<
+  "'%0' will always evaluate to 'true' in a manifestly constant-evaluated expression">,
+  InGroup<DiagGroup<"constant-evaluated">>;
+
 // inline asm related.
 let CategoryName = "Inline Assembly Issue" in {
   def err_asm_invalid_escape : Error<
     "invalid %% escape in inline assembly string">;
   def err_asm_unknown_symbolic_operand_name : Error<
     "unknown symbolic operand name in inline assembly string">;
 
   def err_asm_unterminated_symbolic_operand_name : Error<

clang/lib/AST/ExprConstant.cpp

if (Info.InConstantContext || Arg->HasSideEffects(Info.Ctx)) {
       // of side-effects in order to avoid -Wunsequenced false-positives in
       // a branch on __builtin_constant_p(expr).
       return Success(false, E);
     }
     Info.FFDiag(E, diag::note_invalid_subexpr_in_const_expr);
     return false;
   }
 
-  case Builtin::BI__builtin_is_constant_evaluated:
+  case Builtin::BI__builtin_is_constant_evaluated: {
+    const auto *Callee = Info.CurrentCall->getCallee();
+    if (Info.InConstantContext && !Info.CheckingPotentialConstantExpression &&
+        (Info.CallStackDepth == 1 ||
+         (Info.CallStackDepth == 2 && Callee->isInStdNamespace() &&
+          Callee->getIdentifier() &&
+          Callee->getIdentifier()->isStr("is_constant_evaluated")))) {
+      // FIXME: Find a better way to avoid duplicated diagnostics.
+      if (Info.EvalStatus.Diag)
+        Info.report((Info.CallStackDepth == 1) ? E->getExprLoc()
+                                               : Info.CurrentCall->CallLoc,
+                    diag::warn_is_constant_evaluated_always_true_constexpr)
+            << (Info.CallStackDepth == 1 ? "__builtin_is_constant_evaluated"
+                                         : "std::is_constant_evaluated");
+    }
+
     return Success(Info.InConstantContext, E);
+  }
 
   case Builtin::BI__builtin_ctz:
   case Builtin::BI__builtin_ctzl:
   case Builtin::BI__builtin_ctzll:
   case Builtin::BI__builtin_ctzs: {
     APSInt Val;
     if (!EvaluateInteger(E->getArg(0), Val, Info))
       return false;

clang/test/SemaCXX/builtin-is-constant-evaluated.cpp

-// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -triple=x86_64-linux-gnu
+// RUN: %clang_cc1 -std=c++2a -verify %s -fcxx-exceptions -Wno-constant-evaluated -triple=x86_64-linux-gnu
 
 using size_t = decltype(sizeof(int));
 
 namespace std {
 inline constexpr bool is_constant_evaluated() noexcept {
   return __builtin_is_constant_evaluated();
 }
 } // namespace std

clang/test/SemaCXX/warn-constant-evaluated-constexpr.cpp

+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -verify %s
+
+namespace std {
+constexpr bool is_constant_evaluated() noexcept {
+  return __builtin_is_constant_evaluated();
+}
+} // namespace std
+
+constexpr int fn1() {
+  if constexpr (std::is_constant_evaluated()) // expected-warning {{'std::is_constant_evaluated' will always evaluate to 'true' in a manifestly constant-evaluated expression}}
+    return 0;
+  else
+    return 1;
+}
+
+constexpr int fn2() {
+  if constexpr (!std::is_constant_evaluated()) // expected-warning {{'std::is_constant_evaluated' will always evaluate to 'true' in a manifestly constant-evaluated expression}}
+    return 0;
+  else
+    return 1;
+}
+
+constexpr int fn3() {
+  if constexpr (std::is_constant_evaluated() == false) // expected-warning {{'std::is_constant_evaluated' will always evaluate to 'true' in a manifestly constant-evaluated expression}}
+    return 0;
+  else
+    return 1;
+}
+
+constexpr int fn4() {
+  if constexpr (__builtin_is_constant_evaluated() == true) // expected-warning {{'__builtin_is_constant_evaluated' will always evaluate to 'true' in a manifestly constant-evaluated expression}}
+    return 0;
+  else
+    return 1;
+}
+
+constexpr int fn5() {
+  if constexpr (__builtin_is_constant_evaluated()) // expected-warning {{'__builtin_is_constant_evaluated' will always evaluate to 'true' in a manifestly constant-evaluated expression}}
+    return 0;
+  else
+    return 1;
+}
+
+constexpr int nowarn1() {
+  if (std::is_constant_evaluated())
+    return 0;
+  else
+    return 1;
+}
+
+constexpr int nowarn2() {
+  if (!__builtin_is_constant_evaluated())
+    return 0;
+  else
+    return 1;
+}