[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

12851

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

12851

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

7

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

7

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

- Work around with a clang coverage crash which happens when visiting
   Assert builds may still see assertions triggered from this.
 - Fix a failed assertion due to an invalid source location when trying to form
   a coverage report for an unresolved constructor expression.
   (`#62105 <https://github.com/llvm/llvm-project/issues/62105>`_)
 - Fix defaulted equality operator so that it does not attempt to compare unnamed
   bit-fields. This fixes:
   (`#61355 <https://github.com/llvm/llvm-project/issues/61335>`_) and
   (`#61417 <https://github.com/llvm/llvm-project/issues/61417>`_)
+- 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

public:
   /// statement, check whether we're in a method with a related result
   /// type, and if so, emit a note describing what happened.
   void EmitRelatedResultTypeNoteForReturn(QualType destType);
 
   class ConditionResult {
     Decl *ConditionVar;
     FullExprArg Condition;
     bool Invalid;
-    bool HasKnownValue;
-    bool KnownValue;
+    std::optional<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);
+        }
+      }
+    }
     explicit ConditionResult(bool Invalid)
         : ConditionVar(nullptr), Condition(nullptr), Invalid(Invalid),
-          HasKnownValue(false), KnownValue(false) {}
+          KnownValue(std::nullopt) {}
 
   public:
     ConditionResult() : ConditionResult(false) {}
     bool isInvalid() const { return Invalid; }
     std::pair<VarDecl *, Expr *> get() const {
       return std::make_pair(cast_or_null<VarDecl>(ConditionVar),
                             Condition.get());
     }
-    std::optional<bool> getKnownValue() const {
-      if (!HasKnownValue)
-        return std::nullopt;
-      return KnownValue;
-    }
+    std::optional<bool> getKnownValue() const { return KnownValue; }
   };
   static ConditionResult ConditionError() { return ConditionResult(true); }
 
   enum class ConditionKind {
     Boolean,     ///< A boolean condition, from 'if', 'while', 'for', or 'do'.
     ConstexprIf, ///< A constant boolean condition from 'if constexpr'.
     Switch       ///< An integral condition for a 'switch' statement.
   };

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'?}}
+}
+