[clang] Do not crash after suggesting typo correction to constexpr if condition

[Fznamznon]

Summary

In some cases non-null non-constant yet valid expression may reach point where
ConditionResult is created. For example, typo correction mechanism can return
such expression, so double check before evaluating it.

Fixes https://github.com/llvm/llvm-project/issues/61885

[aaron.ballman]

clang/include/clang/Sema/Sema.h

12852

Oof, this adds quite a bit of expense to this constructor -- isIntegerConstantExpr() is implemented by evaluating the constant expression and throwing away the computed result... which we then recompute when setting KnownValue one line later. It'd be better to only do this computation once and use the computed value to initialize both members.

[Fznamznon]

Rebase, evaluate the expression only only time

[Fznamznon]

clang/include/clang/Sema/Sema.h

12852

Oops, now it is evaluated only one time.
It seems clang's codebase have several places with the same mistake I've made here.

[Fznamznon]

Use std::optional

[shafik]

LGTM

[Fznamznon]

Rebase, fix format

[Fznamznon]

@aaron.ballman , are you ok with the change?

[aaron.ballman]

LGTM!

[zixuan-wu]

clang/test/SemaCXX/invalid-if-constexpr.cpp

8

Hi, why did you mean '__sync_swap' is not reported in riscv target? So it's failed in check-all.

[Fznamznon]

clang/test/SemaCXX/invalid-if-constexpr.cpp

8

I haven't seen any buildbot issues with the issue like this.
Adding -triple riscv64-unknown-unknown doesn't help me to see the issue, as well as adding -DLLVM_TARGETS_TO_BUILD="RISCV" to cmake line and running check-clang. Could you please help to reproduce the issue? I'm not quite familiar with RISCV targets.

clang/docs/ReleaseNotes.rst

 - Clang now better diagnose placeholder types constrained with a concept that is
   not a type concept.
 - Fix crash when a doc comment contains a line splicing.
   (`#62054 <https://github.com/llvm/llvm-project/issues/62054>`_)
 - Work around with a clang coverage crash which happens when visiting 
   expressions/statements with invalid source locations in non-assert builds. 
   Assert builds may still see assertions triggered from this.
   (`#62105 <https://github.com/llvm/llvm-project/issues/62105>`_)
+- Fix crash after suggesting typo correction to constexpr if condition.
+  (`#61885 <https://github.com/llvm/llvm-project/issues/61885>`_)
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Bug Fixes to Attribute Support
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Bug Fixes to C++ Support

clang/include/clang/Sema/Sema.h

class ConditionResult {
     FullExprArg Condition;
     bool Invalid;
     bool HasKnownValue;
     bool KnownValue;
 
     friend class Sema;
     ConditionResult(Sema &S, Decl *ConditionVar, FullExprArg Condition,
                     bool IsConstexpr)
-        : ConditionVar(ConditionVar), Condition(Condition), Invalid(false),
-          HasKnownValue(IsConstexpr && Condition.get() &&
-                        !Condition.get()->isValueDependent()),
-          KnownValue(HasKnownValue &&
-                     !!Condition.get()->EvaluateKnownConstInt(S.Context)) {}
+        : ConditionVar(ConditionVar), Condition(Condition), Invalid(false) {
+      if (IsConstexpr && Condition.get()) {
+        if (std::optional<llvm::APSInt> Val =
+                Condition.get()->getIntegerConstantExpr(S.Context)) {
+          KnownValue = !!(*Val);
+          HasKnownValue = true;
+        }
+      } else {
+        HasKnownValue = false;
+      }
+    }
     explicit ConditionResult(bool Invalid)
         : ConditionVar(nullptr), Condition(nullptr), Invalid(Invalid),
           HasKnownValue(false), KnownValue(false) {}
 
   public:
     ConditionResult() : ConditionResult(false) {}
     bool isInvalid() const { return Invalid; }
     std::pair<VarDecl *, Expr *> get() const {

clang/test/SemaCXX/invalid-if-constexpr.cpp

+// RUN: %clang_cc1 -verify -std=c++20 %s
+
+namespace GH61885 {
+void similar() { // expected-note {{'similar' declared here}}
+  if constexpr (similer<>) {} // expected-error {{use of undeclared identifier 'similer'; did you mean 'similar'?}}
+}
+void a() { if constexpr (__adl_swap<>) {}} // expected-error{{use of undeclared identifier '__adl_swap'; did you mean '__sync_swap'?}} \
+                                           // expected-note {{'__sync_swap' declared here}}
+
+int AA() { return true;} // expected-note {{'AA' declared here}}
+
+void b() { if constexpr (AAA<>) {}} // expected-error {{use of undeclared identifier 'AAA'; did you mean 'AA'?}}
+}
+