[clang] Improve constexpr-unknown diagnostics.

APValue::ConstexprUnknown() constructs a broken LValue that doesn't
have an lvalue path, which confuses later error handling.  It turns out
we don't actually use the result of createConstexprUnknownAPValues for
anything, so just stop using it. Just construct the LValue directly when
we need it.

Make findCompleteObject emit errors more aggressively; allowing it to
succeed for constexpr-unknown objects leads to weird states where it
succeeds, but doesn't return a well-formed object.

Delete the check for constexpr-unknown in dynamic_cast handling: it's
not necessary, and breaks with the other changes in this patch.

These changes allow us to produce proper diagnostics when something
fails to be evaluated, instead of just printing a generic top-level
error without any notes.

Author: efriedma-quic

clang/include/clang/Basic/DiagnosticASTKinds.td

@@ -240,6 +240,8 @@ def note_constexpr_access_static_temporary : Note<
   "outside the expression that created the temporary">;
 def note_constexpr_access_unreadable_object : Note<
   "%sub{access_kind}0 object '%1' whose value is not known">;
+def note_constexpr_access_unknown_variable : Note<
+  "%sub{access_kind}0 variable %1 whose value is not known">;
 def note_constexpr_access_deleted_object : Note<
   "%sub{access_kind}0 heap allocated object that has been deleted">;
 def note_constexpr_modify_global : Note<

clang/lib/AST/ExprConstant.cpp

@@ -202,7 +202,9 @@ namespace {
     assert(!isBaseAnAllocSizeCall(Base) &&
            "Unsized arrays shouldn't appear here");
     unsigned MostDerivedLength = 0;
-    Type = getType(Base);
+    // The type of Base is a reference type if the base is a constexpr-unknown
+    // variable. In that case, look through the reference type.
+    Type = getType(Base).getNonReferenceType();
 
     for (unsigned I = 0, N = Path.size(); I != N; ++I) {
       if (Type->isArrayType()) {
@@ -571,7 +573,6 @@ namespace {
     typedef std::map<MapKeyTy, APValue> MapTy;
     /// Temporaries - Temporary lvalues materialized within this stack frame.
     MapTy Temporaries;
-    MapTy ConstexprUnknownAPValues;
 
     /// CallRange - The source range of the call expression for this call.
     SourceRange CallRange;
@@ -646,9 +647,6 @@ namespace {
     APValue &createTemporary(const KeyT *Key, QualType T,
                              ScopeKind Scope, LValue &LV);
 
-    APValue &createConstexprUnknownAPValues(const VarDecl *Key,
-                                            APValue::LValueBase Base);
-
     /// Allocate storage for a parameter of a function call made in this frame.
     APValue &createParam(CallRef Args, const ParmVarDecl *PVD, LValue &LV);
 
@@ -1955,15 +1953,6 @@ APValue &CallStackFrame::createTemporary(const KeyT *Key, QualType T,
   return createLocal(Base, Key, T, Scope);
 }
 
-APValue &
-CallStackFrame::createConstexprUnknownAPValues(const VarDecl *Key,
-                                               APValue::LValueBase Base) {
-  APValue &Result = ConstexprUnknownAPValues[MapKeyTy(Key, Base.getVersion())];
-  Result = APValue(Base, CharUnits::Zero(), APValue::ConstexprUnknown{});
-
-  return Result;
-}
-
 /// Allocate storage for a parameter of a function call made in this frame.
 APValue &CallStackFrame::createParam(CallRef Args, const ParmVarDecl *PVD,
                                      LValue &LV) {
@@ -3493,7 +3482,7 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
   APValue::LValueBase Base(VD, Frame ? Frame->Index : 0, Version);
 
   auto CheckUninitReference = [&](bool IsLocalVariable) {
-    if (!Result->hasValue() && VD->getType()->isReferenceType()) {
+    if (!Result || (!Result->hasValue() && VD->getType()->isReferenceType())) {
       // C++23 [expr.const]p8
       // ... For such an object that is not usable in constant expressions, the
       // dynamic type of the object is constexpr-unknown. For such a reference
@@ -3509,7 +3498,7 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
           Info.FFDiag(E, diag::note_constexpr_use_uninit_reference);
         return false;
       }
-      Result = &Info.CurrentCall->createConstexprUnknownAPValues(VD, Base);
+      Result = nullptr;
     }
     return true;
   };
@@ -3552,7 +3541,7 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
   // ... its lifetime began within the evaluation of E;
   if (isa<ParmVarDecl>(VD)) {
     if (AllowConstexprUnknown) {
-      Result = &Info.CurrentCall->createConstexprUnknownAPValues(VD, Base);
+      Result = nullptr;
       return true;
     }
 
@@ -3659,12 +3648,8 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
 
   Result = VD->getEvaluatedValue();
 
-  if (!Result) {
-    if (AllowConstexprUnknown)
-      Result = &Info.CurrentCall->createConstexprUnknownAPValues(VD, Base);
-    else
-      return false;
-  }
+  if (!Result && !AllowConstexprUnknown)
+    return false;
 
   return CheckUninitReference(/*IsLocalVariable=*/false);
 }
@@ -3947,11 +3932,6 @@ findSubobject(EvalInfo &Info, const Expr *E, const CompleteObject &Obj,
   const FieldDecl *LastField = nullptr;
   const FieldDecl *VolatileField = nullptr;
 
-  // C++23 [expr.const]p8 If we have an unknown reference or pointers and it
-  // does not have a value then bail out.
-  if (O->allowConstexprUnknown() && !O->hasValue())
-    return false;
-
   // Walk the designator's path to find the subobject.
   for (unsigned I = 0, N = Sub.Entries.size(); /**/; ++I) {
     // Reading an indeterminate value is undefined, but assigning over one is OK.
@@ -4491,6 +4471,14 @@ static CompleteObject findCompleteObject(EvalInfo &Info, const Expr *E,
 
     if (!evaluateVarDeclInit(Info, E, VD, Frame, LVal.getLValueVersion(), BaseVal))
       return CompleteObject();
+    // If evaluateVarDeclInit sees a constexpr-unknown variable, it returns
+    // a null BaseVal. Any constexpr-unknown variable seen here is an error:
+    // we can't access a constexpr-unknown object.
+    if (!BaseVal) {
+      Info.FFDiag(E, diag::note_constexpr_access_unknown_variable, 1) << AK << VD;
+      Info.Note(VD->getLocation(), diag::note_declared_at);
+      return CompleteObject();
+    }
   } else if (DynamicAllocLValue DA = LVal.Base.dyn_cast<DynamicAllocLValue>()) {
     std::optional<DynAlloc *> Alloc = Info.lookupDynamicAlloc(DA);
     if (!Alloc) {
@@ -6057,15 +6045,6 @@ struct CheckDynamicTypeHandler {
 /// dynamic type.
 static bool checkDynamicType(EvalInfo &Info, const Expr *E, const LValue &This,
                              AccessKinds AK, bool Polymorphic) {
-  // We are not allowed to invoke a virtual function whose dynamic type
-  // is constexpr-unknown, so stop early and let this fail later on if we
-  // attempt to do so.
-  // C++23 [expr.const]p5.6
-  // an invocation of a virtual function ([class.virtual]) for an object whose
-  // dynamic type is constexpr-unknown;
-  if (This.allowConstexprUnknown())
-    return true;
-
   if (This.Designator.Invalid)
     return false;
 
@@ -9063,6 +9042,12 @@ bool LValueExprEvaluator::VisitVarDecl(const Expr *E, const VarDecl *VD) {
   if (!evaluateVarDeclInit(Info, E, VD, Frame, Version, V))
     return false;
 
+  if (!V) {
+    Result.set(VD);
+    Result.AllowConstexprUnknown = true;
+    return true;
+  }
+
   return Success(*V, E);
 }
 

clang/test/SemaCXX/constant-expression-cxx11.cpp

@@ -1466,7 +1466,7 @@ namespace InstantiateCaseStmt {
 
 namespace ConvertedConstantExpr {
   extern int &m;
-  extern int &n; // pre-cxx23-note 2{{declared here}}
+  extern int &n; // expected-note 2{{declared here}}
 
   constexpr int k = 4;
   int &m = const_cast<int&>(k);
@@ -1475,9 +1475,9 @@ namespace ConvertedConstantExpr {
   // useless note and instead just point to the non-constant subexpression.
   enum class E {
     em = m,
-    en = n, // expected-error {{enumerator value is not a constant expression}} cxx11_20-note {{initializer of 'n' is unknown}}
+    en = n, // expected-error {{enumerator value is not a constant expression}} cxx11_20-note {{initializer of 'n' is unknown}} cxx23-note {{read of non-constexpr variable 'n'}}
     eo = (m + // expected-error {{not a constant expression}}
-          n // cxx11_20-note {{initializer of 'n' is unknown}}
+          n // cxx11_20-note {{initializer of 'n' is unknown}} cxx23-note {{read of non-constexpr variable 'n'}}
           ),
     eq = reinterpret_cast<long>((int*)0) // expected-error {{not a constant expression}} expected-note {{reinterpret_cast}}
   };

clang/test/SemaCXX/constant-expression-p2280r4.cpp

@@ -35,16 +35,18 @@ constexpr int how_many(Swim& swam) {
   return (p + 1 - 1)->phelps();
 }
 
-void splash(Swim& swam) {
+void splash(Swim& swam) {                 // nointerpreter-note {{declared here}}
   static_assert(swam.phelps() == 28);     // ok
   static_assert((&swam)->phelps() == 28); // ok
   Swim* pswam = &swam;                    // expected-note {{declared here}}
   static_assert(pswam->phelps() == 28);   // expected-error {{static assertion expression is not an integral constant expression}} \
                                           // expected-note {{read of non-constexpr variable 'pswam' is not allowed in a constant expression}}
   static_assert(how_many(swam) == 28);    // ok
   static_assert(Swim().lochte() == 12);   // ok
-  static_assert(swam.lochte() == 12);     // expected-error {{static assertion expression is not an integral constant expression}}
-  static_assert(swam.coughlin == 12);     // expected-error {{static assertion expression is not an integral constant expression}}
+  static_assert(swam.lochte() == 12);     // expected-error {{static assertion expression is not an integral constant expression}} \
+                                          // nointerpreter-note {{virtual function called on object 'swam' whose dynamic type is not constant}}
+  static_assert(swam.coughlin == 12);     // expected-error {{static assertion expression is not an integral constant expression}} \
+                                          // nointerpreter-note {{read of variable 'swam' whose value is not known}}
 }
 
 extern Swim dc;
@@ -253,12 +255,56 @@ namespace uninit_reference_used {
 
 namespace param_reference {
   constexpr int arbitrary = -12345;
-  constexpr void f(const int &x = arbitrary) { // expected-note {{declared here}}
+  constexpr void f(const int &x = arbitrary) { // nointerpreter-note 3 {{declared here}} interpreter-note {{declared here}}
     constexpr const int &v1 = x; // expected-error {{must be initialized by a constant expression}} \
     // expected-note {{reference to 'x' is not a constant expression}}
     constexpr const int &v2 = (x, arbitrary); // expected-warning {{left operand of comma operator has no effect}}
-    constexpr int v3 = x; // expected-error {{must be initialized by a constant expression}}
-    static_assert(x==arbitrary); // expected-error {{static assertion expression is not an integral constant expression}}
+    constexpr int v3 = x; // expected-error {{must be initialized by a constant expression}} \
+                          // nointerpreter-note {{read of variable 'x' whose value is not known}}
+    static_assert(x==arbitrary); // expected-error {{static assertion expression is not an integral constant expression}} \
+                                 // nointerpreter-note {{read of variable 'x' whose value is not known}}
     static_assert(&x - &x == 0);
   }
 }
+
+namespace dropped_note {
+  extern int &x; // expected-note {{declared here}}
+  constexpr int f() { return x; } // nointerpreter-note {{read of non-constexpr variable 'x'}} \
+                                  // interpreter-note {{initializer of 'x' is unknown}}
+  constexpr int y = f(); // expected-error {{constexpr variable 'y' must be initialized by a constant expression}} expected-note {{in call to 'f()'}}
+}
+
+namespace dynamic {
+  struct A {virtual ~A();};
+  struct B : A {};
+  void f(A& a) {
+    constexpr B* b = dynamic_cast<B*>(&a); // expected-error {{must be initialized by a constant expression}} \
+                                           // nointerpreter-note {{dynamic_cast applied to object 'a' whose dynamic type is not constant}}
+    constexpr void* b2 = dynamic_cast<void*>(&a); // expected-error {{must be initialized by a constant expression}} \
+                                                  // nointerpreter-note {{dynamic_cast applied to object 'a' whose dynamic type is not constant}}
+  }
+}
+
+namespace pointer_comparisons {
+  extern int &extern_n; // interpreter-note 2 {{declared here}}
+  extern int &extern_n2;
+  constexpr int f1(bool b, int& n) {
+    if (b) {
+      return &extern_n == &n;
+    }
+    return f1(true, n);
+  }
+  // FIXME: interpreter incorrectly rejects; both sides are the same constexpr-unknown value.
+  static_assert(f1(false, extern_n)); // interpreter-error {{static assertion expression is not an integral constant expression}} \
+                                      // interpreter-note {{initializer of 'extern_n' is unknown}}
+  // FIXME: We should diagnose this: we don't know if the references bind
+  // to the same object.
+  static_assert(&extern_n != &extern_n2); // interpreter-error {{static assertion expression is not an integral constant expression}} \
+                                          // interpreter-note {{initializer of 'extern_n' is unknown}}
+  void f2(const int &n) {
+    // FIXME: We should not diagnose this: the two objects provably have
+    // different addresses because the lifetime of "n" extends across
+    // the initialization.
+    constexpr int x = &x == &n; // nointerpreter-error {{must be initialized by a constant expression}}
+  }
+}