Author: Takuya Shimizu
Date: 2023-05-24T21:31:25+09:00
New Revision: 456d072405d29ac731ad22fa1ec198b9f8265c4e

URL: 
https://github.com/llvm/llvm-project/commit/456d072405d29ac731ad22fa1ec198b9f8265c4e
DIFF: 
https://github.com/llvm/llvm-project/commit/456d072405d29ac731ad22fa1ec198b9f8265c4e.diff

LOG: Reland: [clang][AST] Print name instead of type when diagnosing 
uninitialized subobject in constexpr variables

This patch improves the diagnostic on uninitialized subobjects in constexpr 
variables by modifying the diagnostic message to display the subobject's name 
instead of its type.

Fixes https://github.com/llvm/llvm-project/issues/58601
Differential Revision: https://reviews.llvm.org/D146358

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/include/clang/Basic/DiagnosticASTKinds.td
    clang/lib/AST/ExprConstant.cpp
    clang/lib/AST/Interp/Interp.cpp
    clang/test/AST/Interp/cxx20.cpp
    clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
    clang/test/SemaCXX/constant-expression-cxx2a.cpp
    clang/test/SemaCXX/cxx2a-consteval.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a124617fac8bb..0c369a7f45057 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -296,7 +296,9 @@ Improvements to Clang's diagnostics
   Clang ABI >= 15.
   (`#62353: <https://github.com/llvm/llvm-project/issues/62353>`_,
   fallout from the non-POD packing ABI fix in LLVM 15).
-
+- Clang constexpr evaluator now prints subobject's name instead of its type in 
notes
+  when a constexpr variable has uninitialized subobjects after its constructor 
call.
+  (`#58601 <https://github.com/llvm/llvm-project/issues/58601>`_)
 
 Bug Fixes in This Version
 -------------------------

diff  --git a/clang/include/clang/Basic/DiagnosticASTKinds.td 
b/clang/include/clang/Basic/DiagnosticASTKinds.td
index c283ee842e730..eb13467317963 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -65,7 +65,7 @@ def note_consteval_address_accessible : Note<
   "%select{pointer|reference}0 to a consteval declaration "
   "is not a constant expression">;
 def note_constexpr_uninitialized : Note<
-  "%select{|sub}0object of type %1 is not initialized">;
+  "subobject %0 is not initialized">;
 def note_constexpr_static_local : Note<
   "control flows through the definition of a %select{static|thread_local}0 
variable">;
 def note_constexpr_subobject_declared_here : Note<

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index d88f9535c1a4b..1f1ce2a18bd4a 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2119,7 +2119,7 @@ static bool 
CheckEvaluationResult(CheckEvaluationResultKind CERK,
                                   EvalInfo &Info, SourceLocation DiagLoc,
                                   QualType Type, const APValue &Value,
                                   ConstantExprKind Kind,
-                                  SourceLocation SubobjectLoc,
+                                  const FieldDecl *SubobjectDecl,
                                   CheckedTemporaries &CheckedTemps);
 
 /// Check that this reference or pointer core constant expression is a valid
@@ -2266,8 +2266,8 @@ static bool CheckLValueConstantExpression(EvalInfo &Info, 
SourceLocation Loc,
       APValue *V = MTE->getOrCreateValue(false);
       assert(V && "evasluation result refers to uninitialised temporary");
       if (!CheckEvaluationResult(CheckEvaluationResultKind::ConstantExpression,
-                                 Info, MTE->getExprLoc(), TempType, *V,
-                                 Kind, SourceLocation(), CheckedTemps))
+                                 Info, MTE->getExprLoc(), TempType, *V, Kind,
+                                 /*SubobjectDecl=*/nullptr, CheckedTemps))
         return false;
     }
   }
@@ -2350,13 +2350,13 @@ static bool 
CheckEvaluationResult(CheckEvaluationResultKind CERK,
                                   EvalInfo &Info, SourceLocation DiagLoc,
                                   QualType Type, const APValue &Value,
                                   ConstantExprKind Kind,
-                                  SourceLocation SubobjectLoc,
+                                  const FieldDecl *SubobjectDecl,
                                   CheckedTemporaries &CheckedTemps) {
   if (!Value.hasValue()) {
-    Info.FFDiag(DiagLoc, diag::note_constexpr_uninitialized)
-      << true << Type;
-    if (SubobjectLoc.isValid())
-      Info.Note(SubobjectLoc, diag::note_constexpr_subobject_declared_here);
+    assert(SubobjectDecl && "SubobjectDecl shall be non-null");
+    Info.FFDiag(DiagLoc, diag::note_constexpr_uninitialized) << SubobjectDecl;
+    Info.Note(SubobjectDecl->getLocation(),
+              diag::note_constexpr_subobject_declared_here);
     return false;
   }
 
@@ -2373,20 +2373,19 @@ static bool 
CheckEvaluationResult(CheckEvaluationResultKind CERK,
     for (unsigned I = 0, N = Value.getArrayInitializedElts(); I != N; ++I) {
       if (!CheckEvaluationResult(CERK, Info, DiagLoc, EltTy,
                                  Value.getArrayInitializedElt(I), Kind,
-                                 SubobjectLoc, CheckedTemps))
+                                 SubobjectDecl, CheckedTemps))
         return false;
     }
     if (!Value.hasArrayFiller())
       return true;
     return CheckEvaluationResult(CERK, Info, DiagLoc, EltTy,
-                                 Value.getArrayFiller(), Kind, SubobjectLoc,
+                                 Value.getArrayFiller(), Kind, SubobjectDecl,
                                  CheckedTemps);
   }
   if (Value.isUnion() && Value.getUnionField()) {
     return CheckEvaluationResult(
         CERK, Info, DiagLoc, Value.getUnionField()->getType(),
-        Value.getUnionValue(), Kind, Value.getUnionField()->getLocation(),
-        CheckedTemps);
+        Value.getUnionValue(), Kind, Value.getUnionField(), CheckedTemps);
   }
   if (Value.isStruct()) {
     RecordDecl *RD = Type->castAs<RecordType>()->getDecl();
@@ -2395,7 +2394,7 @@ static bool 
CheckEvaluationResult(CheckEvaluationResultKind CERK,
       for (const CXXBaseSpecifier &BS : CD->bases()) {
         if (!CheckEvaluationResult(CERK, Info, DiagLoc, BS.getType(),
                                    Value.getStructBase(BaseIndex), Kind,
-                                   BS.getBeginLoc(), CheckedTemps))
+                                   /*SubobjectDecl=*/nullptr, CheckedTemps))
           return false;
         ++BaseIndex;
       }
@@ -2405,8 +2404,8 @@ static bool 
CheckEvaluationResult(CheckEvaluationResultKind CERK,
         continue;
 
       if (!CheckEvaluationResult(CERK, Info, DiagLoc, I->getType(),
-                                 Value.getStructField(I->getFieldIndex()),
-                                 Kind, I->getLocation(), CheckedTemps))
+                                 Value.getStructField(I->getFieldIndex()), 
Kind,
+                                 I, CheckedTemps))
         return false;
     }
   }
@@ -2440,7 +2439,7 @@ static bool CheckConstantExpression(EvalInfo &Info, 
SourceLocation DiagLoc,
   CheckedTemporaries CheckedTemps;
   return CheckEvaluationResult(CheckEvaluationResultKind::ConstantExpression,
                                Info, DiagLoc, Type, Value, Kind,
-                               SourceLocation(), CheckedTemps);
+                               /*SubobjectDecl=*/nullptr, CheckedTemps);
 }
 
 /// Check that this evaluated value is fully-initialized and can be loaded by
@@ -2450,7 +2449,7 @@ static bool CheckFullyInitialized(EvalInfo &Info, 
SourceLocation DiagLoc,
   CheckedTemporaries CheckedTemps;
   return CheckEvaluationResult(
       CheckEvaluationResultKind::FullyInitialized, Info, DiagLoc, Type, Value,
-      ConstantExprKind::Normal, SourceLocation(), CheckedTemps);
+      ConstantExprKind::Normal, /*SubobjectDecl=*/nullptr, CheckedTemps);
 }
 
 /// Enforce C++2a [expr.const]/4.17, which disallows new-expressions unless

diff  --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp
index 3af8027ddeb53..4d331467f8f2e 100644
--- a/clang/lib/AST/Interp/Interp.cpp
+++ b/clang/lib/AST/Interp/Interp.cpp
@@ -369,11 +369,11 @@ bool CheckPure(InterpState &S, CodePtr OpPC, const 
CXXMethodDecl *MD) {
 }
 
 static void DiagnoseUninitializedSubobject(InterpState &S, const SourceInfo 
&SI,
-                                           QualType SubObjType,
-                                           SourceLocation SubObjLoc) {
-  S.FFDiag(SI, diag::note_constexpr_uninitialized) << true << SubObjType;
-  if (SubObjLoc.isValid())
-    S.Note(SubObjLoc, diag::note_constexpr_subobject_declared_here);
+                                           const FieldDecl *SubObjDecl) {
+  assert(SubObjDecl && "Subobject declaration does not exist");
+  S.FFDiag(SI, diag::note_constexpr_uninitialized) << SubObjDecl;
+  S.Note(SubObjDecl->getLocation(),
+         diag::note_constexpr_subobject_declared_here);
 }
 
 static bool CheckFieldsInitialized(InterpState &S, CodePtr OpPC,
@@ -400,8 +400,8 @@ static bool CheckArrayInitialized(InterpState &S, CodePtr 
OpPC,
   } else {
     for (size_t I = 0; I != NumElems; ++I) {
       if (!BasePtr.atIndex(I).isInitialized()) {
-        DiagnoseUninitializedSubobject(S, S.Current->getSource(OpPC), ElemType,
-                                       BasePtr.getFieldDesc()->getLocation());
+        DiagnoseUninitializedSubobject(S, S.Current->getSource(OpPC),
+                                       BasePtr.getField());
         Result = false;
       }
     }
@@ -426,8 +426,7 @@ static bool CheckFieldsInitialized(InterpState &S, CodePtr 
OpPC,
           cast<ConstantArrayType>(FieldType->getAsArrayTypeUnsafe());
       Result &= CheckArrayInitialized(S, OpPC, FieldPtr, CAT);
     } else if (!FieldPtr.isInitialized()) {
-      DiagnoseUninitializedSubobject(S, S.Current->getSource(OpPC),
-                                     F.Decl->getType(), F.Decl->getLocation());
+      DiagnoseUninitializedSubobject(S, S.Current->getSource(OpPC), F.Decl);
       Result = false;
     }
   }

diff  --git a/clang/test/AST/Interp/cxx20.cpp b/clang/test/AST/Interp/cxx20.cpp
index 37be7a41bf754..2bf935ef2375b 100644
--- a/clang/test/AST/Interp/cxx20.cpp
+++ b/clang/test/AST/Interp/cxx20.cpp
@@ -143,9 +143,9 @@ namespace UninitializedFields {
     constexpr A() {}
   };
   constexpr A a; // expected-error {{must be initialized by a constant 
expression}} \
-                 // expected-note {{subobject of type 'int' is not 
initialized}} \
+                 // expected-note {{subobject 'a' is not initialized}} \
                  // ref-error {{must be initialized by a constant expression}} 
\
-                 // ref-note {{subobject of type 'int' is not initialized}}
+                 // ref-note {{subobject 'a' is not initialized}}
 
 
   class Base {
@@ -161,18 +161,18 @@ namespace UninitializedFields {
     constexpr Derived() : Base() {}   };
 
   constexpr Derived D; // expected-error {{must be initialized by a constant 
expression}} \
-                       // expected-note {{subobject of type 'int' is not 
initialized}} \
+                       // expected-note {{subobject 'a' is not initialized}} \
                        // ref-error {{must be initialized by a constant 
expression}} \
-                       // ref-note {{subobject of type 'int' is not 
initialized}}
+                       // ref-note {{subobject 'a' is not initialized}}
 
   class C2 {
   public:
     A a;
     constexpr C2() {}   };
   constexpr C2 c2; // expected-error {{must be initialized by a constant 
expression}} \
-                   // expected-note {{subobject of type 'int' is not 
initialized}} \
+                   // expected-note {{subobject 'a' is not initialized}} \
                    // ref-error {{must be initialized by a constant 
expression}} \
-                   // ref-note {{subobject of type 'int' is not initialized}}
+                   // ref-note {{subobject 'a' is not initialized}}
 
 
   // FIXME: These two are currently disabled because the array fields

diff  --git a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp 
b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
index b6f3e2e721f38..f1f677ebfcd34 100644
--- a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
+++ b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
@@ -360,7 +360,7 @@ namespace PR14503 {
   // The constructor is still 'constexpr' here, but the result is not intended
   // to be a constant expression. The standard is not clear on how this should
   // work.
-  constexpr V<int> v; // expected-error {{constant expression}} expected-note 
{{subobject of type 'int' is not initialized}}
+  constexpr V<int> v; // expected-error {{constant expression}} expected-note 
{{subobject 'y' is not initialized}}
 
   constexpr int k = V<int>().x; // FIXME: ok?
 }

diff  --git a/clang/test/SemaCXX/constant-expression-cxx2a.cpp 
b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
index 34881d7446e58..09f17d5b38949 100644
--- a/clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -409,12 +409,12 @@ namespace Union {
     b.a.x = 2;
     return b;
   }
-  constexpr B uninit = return_uninit(); // expected-error {{constant 
expression}} expected-note {{subobject of type 'int' is not initialized}}
+  constexpr B uninit = return_uninit(); // expected-error {{constant 
expression}} expected-note {{subobject 'y' is not initialized}}
   static_assert(return_uninit().a.x == 2);
   constexpr A return_uninit_struct() {
     B b = {.b = 1};
     b.a.x = 2;
-    return b.a; // expected-note {{in call to 'A(b.a)'}} expected-note 
{{subobject of type 'int' is not initialized}}
+    return b.a; // expected-note {{in call to 'A(b.a)'}} expected-note 
{{subobject 'y' is not initialized}}
   }
   // Note that this is rejected even though return_uninit() is accepted, and
   // return_uninit() copies the same stuff wrapped in a union.
@@ -558,7 +558,7 @@ namespace Uninit {
     }
   };
   constinit X x1(true);
-  constinit X x2(false); // expected-error {{constant initializer}} 
expected-note {{constinit}} expected-note {{subobject of type 'int' is not 
initialized}}
+  constinit X x2(false); // expected-error {{constant initializer}} 
expected-note {{constinit}} expected-note {{subobject 'n' is not initialized}}
 
   struct Y {
     struct Z { int n; }; // expected-note {{here}}
@@ -577,7 +577,7 @@ namespace Uninit {
   };
   // FIXME: This is working around clang not implementing DR2026. With that
   // fixed, we should be able to test this without the injected copy.
-  constexpr Y copy(Y y) { return y; } // expected-note {{in call to 'Y(y)'}} 
expected-note {{subobject of type 'int' is not initialized}}
+  constexpr Y copy(Y y) { return y; } // expected-note {{in call to 'Y(y)'}} 
expected-note {{subobject 'n' is not initialized}}
   constexpr Y y1 = copy(Y());
   static_assert(y1.z1.n == 1 && y1.z2.n == 2 && y1.z3.n == 3);
 

diff  --git a/clang/test/SemaCXX/cxx2a-consteval.cpp 
b/clang/test/SemaCXX/cxx2a-consteval.cpp
index 811c048342d7e..7946107ae54f3 100644
--- a/clang/test/SemaCXX/cxx2a-consteval.cpp
+++ b/clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -761,7 +761,7 @@ struct S {
 };
 
 S s1; // expected-error {{call to consteval function 
'NamespaceScopeConsteval::S::S' is not a constant expression}} \
-         expected-note {{subobject of type 'int' is not initialized}}
+         expected-note {{subobject 'Val' is not initialized}}
 
 template <typename Ty>
 struct T {
@@ -770,7 +770,7 @@ struct T {
 };
 
 T<int> t; // expected-error {{call to consteval function 
'NamespaceScopeConsteval::T<int>::T' is not a constant expression}} \
-             expected-note {{subobject of type 'int' is not initialized}}
+             expected-note {{subobject 'Val' is not initialized}}
 
 } // namespace NamespaceScopeConsteval
 


        
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to