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

Tue Oct 29 16:00:42 PDT 2019

xbolva00 marked 3 inline comments as done.
xbolva00 added inline comments.

================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8410
+  "'std::is_constant_evaluated' will always evaluate to "
+  "'true' in constexpr mode">, InGroup<TautologicalCompare>;
 def warn_comparison_bitwise_always : Warning<
----------------
rsmith wrote:
> "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`.
I agree. just... gcc put it under -Wtautological-compare. So should I create a new subgroup of -Wtautological-compare? Or just a new group?

================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:3694
   // FIXME: Return this value to the caller so they don't need to recompute it.
+  checkStdIsConstantEvaluated(*this, CondExpr, IsConstexpr);
   llvm::APSInt Value(/*BitWidth*/1);
----------------
rsmith wrote:
> 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.
Yeah, thanks for the hint!

================
Comment at: clang/test/SemaCXX/warn-std-is-constant-evaluated-constexpr.cpp:2
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -verify -Wtautological-compare %s
+
----------------
rsmith wrote:
> This second `RUN` line doesn't add any value; remove it?
Right, I will remove it.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69518/new/

https://reviews.llvm.org/D69518