[Clang][Interp] Fix the location of uninitialized base warning

[yronglin]

Fix the location of diag::note_constexpr_uninitialized_base, make it same as current interpreter.
This PR does not print type name with namespacethat was used to improve the current interpreter's type dump of base class type.

[llvmbot]

@llvm/pr-subscribers-clang

Author: None (yronglin)

Changes

Fix the location of diag::note_constexpr_uninitialized_base, make it same as current interpreter.

---

Full diff: https://github.com/llvm/llvm-project/pull/100761.diff

3 Files Affected:

• (modified) clang/lib/AST/Interp/EvaluationResult.cpp (+9-11)
• (modified) clang/test/AST/Interp/constexpr-subobj-initialization.cpp (+7-11)
• (modified) clang/test/SemaCXX/constexpr-subobj-initialization.cpp (+4-3)

diff --git a/clang/lib/AST/Interp/EvaluationResult.cpp b/clang/lib/AST/Interp/EvaluationResult.cpp
index 1b255711c7b36..57e12598d12e4 100644
--- a/clang/lib/AST/Interp/EvaluationResult.cpp
+++ b/clang/lib/AST/Interp/EvaluationResult.cpp
@@ -122,22 +122,20 @@ static bool CheckFieldsInitialized(InterpState &S, SourceLocation Loc,
   }
 
   // Check Fields in all bases
-  for (const Record::Base &B : R->bases()) {
+  unsigned BaseIndex = 0;
+  const CXXRecordDecl *CD = dyn_cast<CXXRecordDecl>(R->getDecl());
+  for (const CXXBaseSpecifier &BS : CD->bases()) {
+    const Record::Base &B = *R->getBase(BaseIndex);
     Pointer P = BasePtr.atField(B.Offset);
     if (!P.isInitialized()) {
-      const Descriptor *Desc = BasePtr.getDeclDesc();
-      if (Desc->asDecl())
-        S.FFDiag(BasePtr.getDeclDesc()->asDecl()->getLocation(),
-                 diag::note_constexpr_uninitialized_base)
-            << B.Desc->getType();
-      else
-        S.FFDiag(BasePtr.getDeclDesc()->asExpr()->getExprLoc(),
-                 diag::note_constexpr_uninitialized_base)
-            << B.Desc->getType();
-
+      SourceLocation TypeBeginLoc = BS.getBaseTypeLoc();
+      SourceRange Range(TypeBeginLoc, BS.getEndLoc());
+      S.FFDiag(TypeBeginLoc, diag::note_constexpr_uninitialized_base)
+          << B.Desc->getType() << Range;
       return false;
     }
     Result &= CheckFieldsInitialized(S, Loc, P, B.R);
+    BaseIndex++;
   }
 
   // TODO: Virtual bases
diff --git a/clang/test/AST/Interp/constexpr-subobj-initialization.cpp b/clang/test/AST/Interp/constexpr-subobj-initialization.cpp
index 4976b165468bd..4c067423aedfd 100644
--- a/clang/test/AST/Interp/constexpr-subobj-initialization.cpp
+++ b/clang/test/AST/Interp/constexpr-subobj-initialization.cpp
@@ -14,33 +14,29 @@ struct DelBase {
   constexpr DelBase() = delete; // expected-note {{'DelBase' has been explicitly marked deleted here}}
 };
 
-struct Foo : DelBase {
+struct Foo : DelBase { // expected-note 2{{constructor of base class 'baseclass_uninit::DelBase' is not called}}
   constexpr Foo() {}; // expected-error {{call to deleted constructor of 'DelBase'}}
 };
-constexpr Foo f; // expected-error {{must be initialized by a constant expression}} \
-                 // expected-note {{constructor of base class 'baseclass_uninit::DelBase' is not called}}
+constexpr Foo f; // expected-error {{must be initialized by a constant expression}}
 
 struct Bar : Foo {
   constexpr Bar() {};
 };
-constexpr Bar bar; // expected-error {{must be initialized by a constant expression}} \
-                   // expected-note {{constructor of base class 'baseclass_uninit::DelBase' is not called}}
+constexpr Bar bar; // expected-error {{must be initialized by a constant expression}}
 
 struct Base {};
-struct A : Base {
+struct A : Base { // expected-note {{constructor of base class 'baseclass_uninit::Base' is not called}}
   constexpr A() : value() {} // expected-error {{member initializer 'value' does not name a non-static data member or base class}}
 };
 
-constexpr A a; // expected-error {{must be initialized by a constant expression}} \
-               // expected-note {{constructor of base class 'baseclass_uninit::Base' is not called}}
+constexpr A a; // expected-error {{must be initialized by a constant expression}}
 
 
-struct B : Base {
+struct B : Base { // expected-note {{constructor of base class 'baseclass_uninit::Base' is not called}}
   constexpr B() : {} // expected-error {{expected class member or base class name}}
 };
 
-constexpr B b; // expected-error {{must be initialized by a constant expression}} \
-               // expected-note {{constructor of base class 'baseclass_uninit::Base' is not called}}
+constexpr B b; // expected-error {{must be initialized by a constant expression}}
 } // namespace baseclass_uninit
 
 
diff --git a/clang/test/SemaCXX/constexpr-subobj-initialization.cpp b/clang/test/SemaCXX/constexpr-subobj-initialization.cpp
index cd096a9270937..f0252df1e2ce1 100644
--- a/clang/test/SemaCXX/constexpr-subobj-initialization.cpp
+++ b/clang/test/SemaCXX/constexpr-subobj-initialization.cpp
@@ -1,11 +1,12 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify -fexperimental-new-constant-interpreter %s
 
 namespace baseclass_uninit {
 struct DelBase {
   constexpr DelBase() = delete; // expected-note {{'DelBase' has been explicitly marked deleted here}}
 };
 
-struct Foo : DelBase {  // expected-note 2{{constructor of base class 'DelBase' is not called}}
+struct Foo : DelBase {  // expected-note-re 2{{constructor of base class '{{.*}}DelBase' is not called}}
   constexpr Foo() {}; // expected-error {{call to deleted constructor of 'DelBase'}}
 };
 constexpr Foo f; // expected-error {{must be initialized by a constant expression}}
@@ -15,13 +16,13 @@ struct Bar : Foo {
 constexpr Bar bar; // expected-error {{must be initialized by a constant expression}}
 
 struct Base {};
-struct A : Base { // expected-note {{constructor of base class 'Base' is not called}}
+struct A : Base { // expected-note-re {{constructor of base class '{{.*}}Base' is not called}}
   constexpr A() : value() {} // expected-error {{member initializer 'value' does not name a non-static data member or base class}}
 };
 
 constexpr A a; // expected-error {{must be initialized by a constant expression}}
 
-struct B : Base { // expected-note {{constructor of base class 'Base' is not called}}
+struct B : Base { // expected-note-re {{constructor of base class '{{.*}}Base' is not called}}
   constexpr B() : {} // expected-error {{expected class member or base class name}}
 };
 

[tbaederr]

The three regressions all have similar stack traces:

 #5 0x000000000bba9f4c clang::Redeclarable<clang::TagDecl>::getFirstDecl() /home/fedora/interp-tests/llvm-project/clang/include/clang/AST/Redeclarable.h:216:38
 #6 0x000000000bba9cd5 clang::Redeclarable<clang::TagDecl>::getMostRecentDecl() /home/fedora/interp-tests/llvm-project/clang/include/clang/AST/Redeclarable.h:227:12
 #7 0x000000000bba9a69 clang::RecordDecl::getMostRecentDecl() /home/fedora/interp-tests/llvm-project/clang/include/clang/AST/Decl.h:4168:59
 #8 0x000000000bba99b5 clang::CXXRecordDecl::getMostRecentDecl() /home/fedora/interp-tests/llvm-project/clang/include/clang/AST/DeclCXX.h:542:46
 #9 0x000000000bba9995 clang::CXXRecordDecl::getMostRecentDecl() const /home/fedora/interp-tests/llvm-project/clang/include/clang/AST/DeclCXX.h:546:5
#10 0x000000000bba9969 clang::CXXRecordDecl::dataPtr() const /home/fedora/interp-tests/llvm-project/clang/include/clang/AST/DeclCXX.h:458:5
#11 0x000000000bba9885 clang::CXXRecordDecl::data() const /home/fedora/interp-tests/llvm-project/clang/include/clang/AST/DeclCXX.h:463:11
#12 0x000000000bc92be5 clang::CXXRecordDecl::bases_begin() const /home/fedora/interp-tests/llvm-project/clang/include/clang/AST/DeclCXX.h:627:58
#13 0x000000000bc7a6a9 clang::CXXRecordDecl::bases() const /home/fedora/interp-tests/llvm-project/clang/include/clang/AST/DeclCXX.h:623:35
#14 0x00000000127e9d9f clang::interp::CheckFieldsInitialized(clang::interp::InterpState&, clang::SourceLocation, clang::interp::Pointer const&, clang::interp::Record const*) /home/fedora/interp-tests/llvm-project/clang/lib/AST/Interp/EvaluationResult.cpp:127:41

So I think it's cases where the decl is no CXXRecordDecl.
This patch also drops the handling where the pointer descriptor was created for an expression - this might be relevant for those tests or not.

[yronglin]

Thanks for the review! Yes, there are three regressions, I've revert back the handling of an expression, and enable new interpreter in these three test.

[tbaederr]

CC @AaronBallman @Sirraide @shafik Since this adds new API to CXXBaseSpecifier.

[Sirraide]

CXXRecordDecl already exposes bases() and vbases(), I don’t think we need this new API for accessing a base by index. This pr looks like it should just be using llvm::enumerate.

[yronglin]

CXXRecordDecl already exposes bases() and vbases(), I don’t think we need this new API for accessing a base by index. This pr looks like it should just be using llvm::enumerate.

Thanks for the review! I've tried to use llvm::enumerate(clang::interp::Record->bases(), clang::CXXRecordDecl->bases()), but clang::CXXRecordDecl may be null in some case(clang::interp::Record was not a CXXRecordDecl). IIUC, if clang::CXXRecordDecl is null, we need only traverse clang::interp::Record->bases(). I've removed these two APIs in CXXRecordDecl, and use std::next instead, WDYT?

[Sirraide]

I've removed these two APIs in CXXRecordDecl, and use std::next instead, WDYT?

Makes sense; that’s more or less what I had in mind too.

[Sirraide]

Alright, that addresses all the changes I was concerned about, so this lgtm, but I’ll leave it to @tbaederr to approve this for good since I’m not really familiar w/ the interpreter...

[yronglin]

@tbaederr @Sirraide Thanks for your review!