[PATCH] D105127: Implement P1401R5

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

P1401R5 points out that we get `noexcept(bool)` wrong in the opposite direction, permitting conversions to `bool` that we are supposed to reject. While that rule didn't change as part of P1401R5, it'd be nice to handle it as part of dealing with that paper.



================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:3929
+
+  if (IsConstexpr && !LangOpts.CPlusPlus2b && !CondExpr->isValueDependent()) {
+    llvm::APSInt Value(/*BitWidth*/ 1);
----------------
I think we should apply this retroactively, even though it wasn't moved as a DR. All other implementations already behave this way across all language modes, and it's just not reasonable to reject conversions in the condition of an `if constexpr` that are accepted in the condition of an `if` -- this seems like an obvious language defect even if the C++ committee haven't officially voted it to be one yet.

Separately I've asked on the committee reflectors if we can officially treat this as a DR, given the implementation consensus.


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:3935
+  ExprResult E = PerformContextuallyConvertToBool(CondExpr);
+  if (!IsConstexpr || !E.isUsable() || E.get()->isValueDependent())
+    return E;
----------------
Please use `E.isInvalid()` instead of `!E.isUsable()` unless you anticipate and intend to handle the valid-but-null case (which is unusable but not invalid).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105127