[clang] [clang-tools-extra] [Clang][Sema] Diagnose current instantiation used as an incomplete base class (PR #92597)

Krystian Stasiowski via cfe-commits cfe-commits at lists.llvm.org
Mon May 20 07:52:07 PDT 2024


https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/92597

>From 4a535c2f2660583487018f421788cd2b88d8428d Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Fri, 17 May 2024 13:30:04 -0400
Subject: [PATCH 1/5] [Clang][Sema] Diagnose current instantiation used a
 incomplete base class

---
 clang/lib/AST/Type.cpp                        |   4 +
 clang/lib/Sema/SemaDeclCXX.cpp                | 116 +++++++++---------
 .../basic.lookup.qual/class.qual/p2.cpp       |  10 +-
 clang/test/SemaTemplate/dependent-names.cpp   |   9 +-
 .../test/SemaTemplate/destructor-template.cpp |  14 ++-
 .../test/SemaTemplate/typo-dependent-name.cpp |   7 +-
 6 files changed, 84 insertions(+), 76 deletions(-)

diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index e31741cd44240..4ad764b67f81f 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -2372,6 +2372,10 @@ bool Type::isIncompleteType(NamedDecl **Def) const {
       *Def = Rec;
     return !Rec->isCompleteDefinition();
   }
+  case InjectedClassName: {
+    CXXRecordDecl *Rec = cast<InjectedClassNameType>(CanonicalType)->getDecl();
+    return Rec->isBeingDefined();
+  }
   case ConstantArray:
   case VariableArray:
     // An array is incomplete if its element type is incomplete
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 8225381985052..84033a602f131 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -2704,28 +2704,53 @@ Sema::CheckBaseSpecifier(CXXRecordDecl *Class,
     Access = AS_public;
 
   QualType BaseType = TInfo->getType();
+  SourceLocation BaseLoc = TInfo->getTypeLoc().getBeginLoc();
   if (BaseType->containsErrors()) {
     // Already emitted a diagnostic when parsing the error type.
     return nullptr;
   }
-  // C++ [class.union]p1:
-  //   A union shall not have base classes.
-  if (Class->isUnion()) {
-    Diag(Class->getLocation(), diag::err_base_clause_on_union)
-      << SpecifierRange;
-    return nullptr;
-  }
 
-  if (EllipsisLoc.isValid() &&
-      !TInfo->getType()->containsUnexpandedParameterPack()) {
+  if (EllipsisLoc.isValid() && !BaseType->containsUnexpandedParameterPack()) {
     Diag(EllipsisLoc, diag::err_pack_expansion_without_parameter_packs)
       << TInfo->getTypeLoc().getSourceRange();
     EllipsisLoc = SourceLocation();
   }
 
-  SourceLocation BaseLoc = TInfo->getTypeLoc().getBeginLoc();
+  auto *BaseDecl =
+      dyn_cast_if_present<CXXRecordDecl>(computeDeclContext(BaseType));
+  // C++ [class.derived.general]p2:
+  //   A class-or-decltype shall denote a (possibly cv-qualified) class type
+  //   that is not an incompletely defined class; any cv-qualifiers are
+  //   ignored.
+  if (BaseDecl) {
+    // C++ [class.union.general]p4:
+    // [...]  A union shall not be used as a base class.
+    if (BaseDecl->isUnion()) {
+      Diag(BaseLoc, diag::err_union_as_base_class) << SpecifierRange;
+      return nullptr;
+    }
+
+    // For the MS ABI, propagate DLL attributes to base class templates.
+    if (Context.getTargetInfo().getCXXABI().isMicrosoft() ||
+        Context.getTargetInfo().getTriple().isPS()) {
+      if (Attr *ClassAttr = getDLLAttr(Class)) {
+        if (auto *BaseSpec =
+                dyn_cast<ClassTemplateSpecializationDecl>(BaseDecl)) {
+          propagateDLLAttrToBaseClassTemplate(Class, ClassAttr, BaseSpec,
+                                              BaseLoc);
+        }
+      }
+    }
 
-  if (BaseType->isDependentType()) {
+    if (RequireCompleteType(BaseLoc, BaseType, diag::err_incomplete_base_class,
+                            SpecifierRange)) {
+      Class->setInvalidDecl();
+      return nullptr;
+    }
+  } else if (!BaseType->isDependentType()) {
+    Diag(BaseLoc, diag::err_base_must_be_class) << SpecifierRange;
+    return nullptr;
+  } else {
     // Make sure that we don't have circular inheritance among our dependent
     // bases. For non-dependent bases, the check for completeness below handles
     // this.
@@ -2750,65 +2775,28 @@ Sema::CheckBaseSpecifier(CXXRecordDecl *Class,
     // constexpr evaluator). If this case happens (in errory-recovery mode), we
     // explicitly mark the Class decl invalid. The diagnostic was already
     // emitted.
-    if (!Class->getTypeForDecl()->isDependentType())
+    if (!Class->isDependentContext())
       Class->setInvalidDecl();
     return new (Context) CXXBaseSpecifier(
         SpecifierRange, Virtual, Class->getTagKind() == TagTypeKind::Class,
         Access, TInfo, EllipsisLoc);
   }
 
-  // Base specifiers must be record types.
-  if (!BaseType->isRecordType()) {
-    Diag(BaseLoc, diag::err_base_must_be_class) << SpecifierRange;
-    return nullptr;
-  }
-
-  // C++ [class.union]p1:
-  //   A union shall not be used as a base class.
-  if (BaseType->isUnionType()) {
-    Diag(BaseLoc, diag::err_union_as_base_class) << SpecifierRange;
-    return nullptr;
-  }
-
-  // For the MS ABI, propagate DLL attributes to base class templates.
-  if (Context.getTargetInfo().getCXXABI().isMicrosoft() ||
-      Context.getTargetInfo().getTriple().isPS()) {
-    if (Attr *ClassAttr = getDLLAttr(Class)) {
-      if (auto *BaseTemplate = dyn_cast_or_null<ClassTemplateSpecializationDecl>(
-              BaseType->getAsCXXRecordDecl())) {
-        propagateDLLAttrToBaseClassTemplate(Class, ClassAttr, BaseTemplate,
-                                            BaseLoc);
-      }
-    }
-  }
-
-  // C++ [class.derived]p2:
-  //   The class-name in a base-specifier shall not be an incompletely
-  //   defined class.
-  if (RequireCompleteType(BaseLoc, BaseType,
-                          diag::err_incomplete_base_class, SpecifierRange)) {
-    Class->setInvalidDecl();
-    return nullptr;
-  }
-
   // If the base class is polymorphic or isn't empty, the new one is/isn't, too.
-  RecordDecl *BaseDecl = BaseType->castAs<RecordType>()->getDecl();
   assert(BaseDecl && "Record type has no declaration");
   BaseDecl = BaseDecl->getDefinition();
   assert(BaseDecl && "Base type is not incomplete, but has no definition");
-  CXXRecordDecl *CXXBaseDecl = cast<CXXRecordDecl>(BaseDecl);
-  assert(CXXBaseDecl && "Base type is not a C++ type");
 
   // Microsoft docs say:
   // "If a base-class has a code_seg attribute, derived classes must have the
   // same attribute."
-  const auto *BaseCSA = CXXBaseDecl->getAttr<CodeSegAttr>();
+  const auto *BaseCSA = BaseDecl->getAttr<CodeSegAttr>();
   const auto *DerivedCSA = Class->getAttr<CodeSegAttr>();
   if ((DerivedCSA || BaseCSA) &&
       (!BaseCSA || !DerivedCSA || BaseCSA->getName() != DerivedCSA->getName())) {
     Diag(Class->getLocation(), diag::err_mismatched_code_seg_base);
-    Diag(CXXBaseDecl->getLocation(), diag::note_base_class_specified_here)
-      << CXXBaseDecl;
+    Diag(BaseDecl->getLocation(), diag::note_base_class_specified_here)
+        << BaseDecl;
     return nullptr;
   }
 
@@ -2818,21 +2806,20 @@ Sema::CheckBaseSpecifier(CXXRecordDecl *Class,
   //     the flexible array member would index into the subsequent base.
   //   - If the layout determines that base comes before the derived class,
   //     the flexible array member would index into the derived class.
-  if (CXXBaseDecl->hasFlexibleArrayMember()) {
+  if (BaseDecl->hasFlexibleArrayMember()) {
     Diag(BaseLoc, diag::err_base_class_has_flexible_array_member)
-      << CXXBaseDecl->getDeclName();
+        << BaseDecl->getDeclName();
     return nullptr;
   }
 
   // C++ [class]p3:
   //   If a class is marked final and it appears as a base-type-specifier in
   //   base-clause, the program is ill-formed.
-  if (FinalAttr *FA = CXXBaseDecl->getAttr<FinalAttr>()) {
+  if (FinalAttr *FA = BaseDecl->getAttr<FinalAttr>()) {
     Diag(BaseLoc, diag::err_class_marked_final_used_as_base)
-      << CXXBaseDecl->getDeclName()
-      << FA->isSpelledAsSealed();
-    Diag(CXXBaseDecl->getLocation(), diag::note_entity_declared_at)
-        << CXXBaseDecl->getDeclName() << FA->getRange();
+        << BaseDecl->getDeclName() << FA->isSpelledAsSealed();
+    Diag(BaseDecl->getLocation(), diag::note_entity_declared_at)
+        << BaseDecl->getDeclName() << FA->getRange();
     return nullptr;
   }
 
@@ -2887,13 +2874,20 @@ BaseResult Sema::ActOnBaseSpecifier(Decl *classdecl, SourceRange SpecifierRange,
                                       UPPC_BaseType))
     return true;
 
+  // C++ [class.union.general]p4:
+  //   [...] A union shall not have base classes.
+  if (Class->isUnion()) {
+    Diag(Class->getLocation(), diag::err_base_clause_on_union)
+        << SpecifierRange;
+    return true;
+  }
+
   if (CXXBaseSpecifier *BaseSpec = CheckBaseSpecifier(Class, SpecifierRange,
                                                       Virtual, Access, TInfo,
                                                       EllipsisLoc))
     return BaseSpec;
-  else
-    Class->setInvalidDecl();
 
+  Class->setInvalidDecl();
   return true;
 }
 
diff --git a/clang/test/CXX/basic/basic.lookup/basic.lookup.qual/class.qual/p2.cpp b/clang/test/CXX/basic/basic.lookup/basic.lookup.qual/class.qual/p2.cpp
index be07ab0a48b33..0fa98ad101f6c 100644
--- a/clang/test/CXX/basic/basic.lookup/basic.lookup.qual/class.qual/p2.cpp
+++ b/clang/test/CXX/basic/basic.lookup/basic.lookup.qual/class.qual/p2.cpp
@@ -141,11 +141,15 @@ namespace InhCtor {
   // ill-formed.
   template<typename T>
   struct S : T {
-    struct U : S { // expected-note 6{{candidate}}
-      using S::S;
-    };
+    struct U; // expected-note 6{{candidate}}
     using T::T;
   };
+
+  template<typename T>
+  struct S<T>::U : S {
+    using S::S;
+  };
+
   S<A>::U ua(0); // expected-error {{no match}}
   S<B>::U ub(0); // expected-error {{no match}}
 
diff --git a/clang/test/SemaTemplate/dependent-names.cpp b/clang/test/SemaTemplate/dependent-names.cpp
index 641ec950054f5..58d3d3eb4a6ff 100644
--- a/clang/test/SemaTemplate/dependent-names.cpp
+++ b/clang/test/SemaTemplate/dependent-names.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s 
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
 
 typedef double A;
 template<typename T> class B {
@@ -334,8 +334,8 @@ int arr[sizeof(Sub)];
 namespace PR11421 {
 template < unsigned > struct X {
   static const unsigned dimension = 3;
-  template<unsigned dim=dimension> 
-  struct Y: Y<dim> { }; // expected-error{{circular inheritance between 'Y<dim>' and 'Y<dim>'}}
+  template<unsigned dim=dimension>
+  struct Y: Y<dim> { }; // expected-error{{base class has incomplete type}}
 };
 typedef X<3> X3;
 X3::Y<>::iterator it; // expected-error {{no type named 'iterator' in 'PR11421::X<3>::Y<>'}}
@@ -344,7 +344,8 @@ X3::Y<>::iterator it; // expected-error {{no type named 'iterator' in 'PR11421::
 namespace rdar12629723 {
   template<class T>
   struct X {
-    struct C : public C { }; // expected-error{{circular inheritance between 'C' and 'rdar12629723::X::C'}}
+    struct C : public C { }; // expected-error{{base class has incomplete type}}
+                             // expected-note at -1{{definition of 'rdar12629723::X::C' is not complete until the closing '}'}}
 
     struct B;
 
diff --git a/clang/test/SemaTemplate/destructor-template.cpp b/clang/test/SemaTemplate/destructor-template.cpp
index 8901882947623..7a3398308bbee 100644
--- a/clang/test/SemaTemplate/destructor-template.cpp
+++ b/clang/test/SemaTemplate/destructor-template.cpp
@@ -1,12 +1,14 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
 
 template<typename A> class s0 {
+  template<typename B> class s1;
+};
 
-  template<typename B> class s1 : public s0<A> {
-    ~s1() {}
-    s0<A> ms0;
-  };
-
+template<typename A>
+template<typename B>
+class s0<A>::s1 : s0<A> {
+  ~s1() {}
+  s0<A> ms0;
 };
 
 struct Incomplete;
@@ -28,7 +30,7 @@ namespace PR6152 {
     y->template Y<T>::~Y<T>();
     y->~Y();
   }
-  
+
   template struct X<int>;
 }
 
diff --git a/clang/test/SemaTemplate/typo-dependent-name.cpp b/clang/test/SemaTemplate/typo-dependent-name.cpp
index fb61b03e50109..5bd924241480d 100644
--- a/clang/test/SemaTemplate/typo-dependent-name.cpp
+++ b/clang/test/SemaTemplate/typo-dependent-name.cpp
@@ -31,8 +31,7 @@ struct Y {
   static int z;
 
   template<int U>
-  struct Inner : Y { // expected-note {{declared here}}
-  };
+  struct Inner; // expected-note {{declared here}}
 
   bool f(T other) {
     // We can determine that 'inner' does not exist at parse time, so can
@@ -41,5 +40,9 @@ struct Y {
   }
 };
 
+template<typename T>
+template<int U>
+struct Y<T>::Inner : Y { };
+
 struct Q { constexpr operator int() { return 0; } };
 void use_y(Y<Q> x) { x.f(Q()); }

>From 9e07409ec8f2a3a35bc24d20529a56e558550d06 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Fri, 17 May 2024 13:52:56 -0400
Subject: [PATCH 2/5] [FOLD] refactor

---
 clang/lib/Sema/SemaDeclCXX.cpp | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 84033a602f131..6bd77c0fec2ae 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -2710,14 +2710,14 @@ Sema::CheckBaseSpecifier(CXXRecordDecl *Class,
     return nullptr;
   }
 
-  if (EllipsisLoc.isValid() && !BaseType->containsUnexpandedParameterPack()) {
+  if (EllipsisLoc.isValid() &&
+      !BaseType->containsUnexpandedParameterPack()) {
     Diag(EllipsisLoc, diag::err_pack_expansion_without_parameter_packs)
       << TInfo->getTypeLoc().getSourceRange();
     EllipsisLoc = SourceLocation();
   }
 
-  auto *BaseDecl =
-      dyn_cast_if_present<CXXRecordDecl>(computeDeclContext(BaseType));
+  auto *BaseDecl = dyn_cast_if_present<CXXRecordDecl>(computeDeclContext(BaseType));
   // C++ [class.derived.general]p2:
   //   A class-or-decltype shall denote a (possibly cv-qualified) class type
   //   that is not an incompletely defined class; any cv-qualifiers are
@@ -2734,23 +2734,18 @@ Sema::CheckBaseSpecifier(CXXRecordDecl *Class,
     if (Context.getTargetInfo().getCXXABI().isMicrosoft() ||
         Context.getTargetInfo().getTriple().isPS()) {
       if (Attr *ClassAttr = getDLLAttr(Class)) {
-        if (auto *BaseSpec =
-                dyn_cast<ClassTemplateSpecializationDecl>(BaseDecl)) {
-          propagateDLLAttrToBaseClassTemplate(Class, ClassAttr, BaseSpec,
-                                              BaseLoc);
+        if (auto *BaseSpec = dyn_cast<ClassTemplateSpecializationDecl>(BaseDecl)) {
+          propagateDLLAttrToBaseClassTemplate(Class, ClassAttr, BaseSpec, BaseLoc);
         }
       }
     }
 
-    if (RequireCompleteType(BaseLoc, BaseType, diag::err_incomplete_base_class,
-                            SpecifierRange)) {
+    if (RequireCompleteType(BaseLoc, BaseType,
+                            diag::err_incomplete_base_class, SpecifierRange)) {
       Class->setInvalidDecl();
       return nullptr;
     }
-  } else if (!BaseType->isDependentType()) {
-    Diag(BaseLoc, diag::err_base_must_be_class) << SpecifierRange;
-    return nullptr;
-  } else {
+  } else if (BaseType->isDependentType()) {
     // Make sure that we don't have circular inheritance among our dependent
     // bases. For non-dependent bases, the check for completeness below handles
     // this.
@@ -2768,7 +2763,6 @@ Sema::CheckBaseSpecifier(CXXRecordDecl *Class,
         return nullptr;
       }
     }
-
     // Make sure that we don't make an ill-formed AST where the type of the
     // Class is non-dependent and its attached base class specifier is an
     // dependent type, which violates invariants in many clang code paths (e.g.
@@ -2780,10 +2774,11 @@ Sema::CheckBaseSpecifier(CXXRecordDecl *Class,
     return new (Context) CXXBaseSpecifier(
         SpecifierRange, Virtual, Class->getTagKind() == TagTypeKind::Class,
         Access, TInfo, EllipsisLoc);
+  } else {
+    Diag(BaseLoc, diag::err_base_must_be_class) << SpecifierRange;
+    return nullptr;
   }
 
-  // If the base class is polymorphic or isn't empty, the new one is/isn't, too.
-  assert(BaseDecl && "Record type has no declaration");
   BaseDecl = BaseDecl->getDefinition();
   assert(BaseDecl && "Base type is not incomplete, but has no definition");
 
@@ -2796,7 +2791,7 @@ Sema::CheckBaseSpecifier(CXXRecordDecl *Class,
       (!BaseCSA || !DerivedCSA || BaseCSA->getName() != DerivedCSA->getName())) {
     Diag(Class->getLocation(), diag::err_mismatched_code_seg_base);
     Diag(BaseDecl->getLocation(), diag::note_base_class_specified_here)
-        << BaseDecl;
+      << BaseDecl;
     return nullptr;
   }
 
@@ -2808,7 +2803,7 @@ Sema::CheckBaseSpecifier(CXXRecordDecl *Class,
   //     the flexible array member would index into the derived class.
   if (BaseDecl->hasFlexibleArrayMember()) {
     Diag(BaseLoc, diag::err_base_class_has_flexible_array_member)
-        << BaseDecl->getDeclName();
+      << BaseDecl->getDeclName();
     return nullptr;
   }
 
@@ -2817,7 +2812,8 @@ Sema::CheckBaseSpecifier(CXXRecordDecl *Class,
   //   base-clause, the program is ill-formed.
   if (FinalAttr *FA = BaseDecl->getAttr<FinalAttr>()) {
     Diag(BaseLoc, diag::err_class_marked_final_used_as_base)
-        << BaseDecl->getDeclName() << FA->isSpelledAsSealed();
+      << BaseDecl->getDeclName()
+      << FA->isSpelledAsSealed();
     Diag(BaseDecl->getLocation(), diag::note_entity_declared_at)
         << BaseDecl->getDeclName() << FA->getRange();
     return nullptr;

>From e4f9615c01a542da4b15820e65010b9c4bf980dc Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Fri, 17 May 2024 15:09:23 -0400
Subject: [PATCH 3/5] [FOLD] provide definition of injected class name while
 type is being defined

---
 clang/lib/AST/Type.cpp                        |  7 +-
 clang/lib/Sema/SemaDeclCXX.cpp                | 23 ++---
 .../class.derived.general/p2.cpp              | 84 +++++++++++++++++++
 clang/test/SemaTemplate/dependent-names.cpp   |  1 +
 4 files changed, 103 insertions(+), 12 deletions(-)
 create mode 100644 clang/test/CXX/class.derived/class.derived.general/p2.cpp

diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 4ad764b67f81f..545725d1d7511 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -2374,7 +2374,12 @@ bool Type::isIncompleteType(NamedDecl **Def) const {
   }
   case InjectedClassName: {
     CXXRecordDecl *Rec = cast<InjectedClassNameType>(CanonicalType)->getDecl();
-    return Rec->isBeingDefined();
+    if (Rec->isBeingDefined()) {
+      if (Def)
+        *Def = Rec;
+      return true;
+    }
+    return false;
   }
   case ConstantArray:
   case VariableArray:
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 6bd77c0fec2ae..f98339fa5748d 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -2710,14 +2710,14 @@ Sema::CheckBaseSpecifier(CXXRecordDecl *Class,
     return nullptr;
   }
 
-  if (EllipsisLoc.isValid() &&
-      !BaseType->containsUnexpandedParameterPack()) {
+  if (EllipsisLoc.isValid() && !BaseType->containsUnexpandedParameterPack()) {
     Diag(EllipsisLoc, diag::err_pack_expansion_without_parameter_packs)
       << TInfo->getTypeLoc().getSourceRange();
     EllipsisLoc = SourceLocation();
   }
 
-  auto *BaseDecl = dyn_cast_if_present<CXXRecordDecl>(computeDeclContext(BaseType));
+  auto *BaseDecl =
+      dyn_cast_if_present<CXXRecordDecl>(computeDeclContext(BaseType));
   // C++ [class.derived.general]p2:
   //   A class-or-decltype shall denote a (possibly cv-qualified) class type
   //   that is not an incompletely defined class; any cv-qualifiers are
@@ -2734,14 +2734,16 @@ Sema::CheckBaseSpecifier(CXXRecordDecl *Class,
     if (Context.getTargetInfo().getCXXABI().isMicrosoft() ||
         Context.getTargetInfo().getTriple().isPS()) {
       if (Attr *ClassAttr = getDLLAttr(Class)) {
-        if (auto *BaseSpec = dyn_cast<ClassTemplateSpecializationDecl>(BaseDecl)) {
-          propagateDLLAttrToBaseClassTemplate(Class, ClassAttr, BaseSpec, BaseLoc);
+        if (auto *BaseSpec =
+                dyn_cast<ClassTemplateSpecializationDecl>(BaseDecl)) {
+          propagateDLLAttrToBaseClassTemplate(Class, ClassAttr, BaseSpec,
+                                              BaseLoc);
         }
       }
     }
 
-    if (RequireCompleteType(BaseLoc, BaseType,
-                            diag::err_incomplete_base_class, SpecifierRange)) {
+    if (RequireCompleteType(BaseLoc, BaseType, diag::err_incomplete_base_class,
+                            SpecifierRange)) {
       Class->setInvalidDecl();
       return nullptr;
     }
@@ -2791,7 +2793,7 @@ Sema::CheckBaseSpecifier(CXXRecordDecl *Class,
       (!BaseCSA || !DerivedCSA || BaseCSA->getName() != DerivedCSA->getName())) {
     Diag(Class->getLocation(), diag::err_mismatched_code_seg_base);
     Diag(BaseDecl->getLocation(), diag::note_base_class_specified_here)
-      << BaseDecl;
+        << BaseDecl;
     return nullptr;
   }
 
@@ -2803,7 +2805,7 @@ Sema::CheckBaseSpecifier(CXXRecordDecl *Class,
   //     the flexible array member would index into the derived class.
   if (BaseDecl->hasFlexibleArrayMember()) {
     Diag(BaseLoc, diag::err_base_class_has_flexible_array_member)
-      << BaseDecl->getDeclName();
+        << BaseDecl->getDeclName();
     return nullptr;
   }
 
@@ -2812,8 +2814,7 @@ Sema::CheckBaseSpecifier(CXXRecordDecl *Class,
   //   base-clause, the program is ill-formed.
   if (FinalAttr *FA = BaseDecl->getAttr<FinalAttr>()) {
     Diag(BaseLoc, diag::err_class_marked_final_used_as_base)
-      << BaseDecl->getDeclName()
-      << FA->isSpelledAsSealed();
+        << BaseDecl->getDeclName() << FA->isSpelledAsSealed();
     Diag(BaseDecl->getLocation(), diag::note_entity_declared_at)
         << BaseDecl->getDeclName() << FA->getRange();
     return nullptr;
diff --git a/clang/test/CXX/class.derived/class.derived.general/p2.cpp b/clang/test/CXX/class.derived/class.derived.general/p2.cpp
new file mode 100644
index 0000000000000..0930375a45737
--- /dev/null
+++ b/clang/test/CXX/class.derived/class.derived.general/p2.cpp
@@ -0,0 +1,84 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify
+
+namespace CurrentInstantiation {
+  template<typename T>
+  struct A0 { // expected-note 6{{definition of 'A0<T>' is not complete until the closing '}'}}
+    struct B0 : A0 { }; // expected-error {{base class has incomplete type}}
+
+    template<typename U>
+    struct B1 : A0 { }; // expected-error {{base class has incomplete type}}
+
+    struct B2;
+
+    template<typename U>
+    struct B3;
+
+    struct B4 { // expected-note 2{{definition of 'CurrentInstantiation::A0::B4' is not complete until the closing '}'}}
+      struct C0 : A0, B4 { }; // expected-error 2{{base class has incomplete type}}
+
+      template<typename V>
+      struct C1 : A0, B4 { }; // expected-error 2{{base class has incomplete type}}
+
+      struct C2;
+
+      template<typename V>
+      struct C3;
+    };
+
+    template<typename U>
+    struct B5 { // expected-note 2{{definition of 'B5<U>' is not complete until the closing '}'}}
+      struct C0 : A0, B5 { }; // expected-error 2{{base class has incomplete type}}
+
+      template<typename V>
+      struct C1 : A0, B5 { }; // expected-error 2{{base class has incomplete type}}
+
+      struct C2;
+
+      template<typename V>
+      struct C3;
+    };
+  };
+
+  template<typename T>
+  struct A0<T>::B2 : A0 { };
+
+  template<typename T>
+  template<typename U>
+  struct A0<T>::B3 : A0 { };
+
+  template<typename T>
+  struct A0<T>::B4::C2 : A0, B4 { };
+
+  template<typename T>
+  template<typename V>
+  struct A0<T>::B4::C3 : A0, B4 { };
+
+  template<typename T>
+  template<typename U>
+  struct A0<T>::B5<U>::C2 : A0, B5 { };
+
+  template<typename T>
+  template<typename U>
+  template<typename V>
+  struct A0<T>::B5<U>::C3 : A0, B5 { };
+
+  template<typename T>
+  struct A0<T*> { // expected-note 2{{definition of 'A0<type-parameter-0-0 *>' is not complete until the closing '}'}}
+    struct B0 : A0 { }; // expected-error {{base class has incomplete type}}
+
+    template<typename U>
+    struct B1 : A0 { }; // expected-error {{base class has incomplete type}}
+
+    struct B2;
+
+    template<typename U>
+    struct B3;
+  };
+
+  template<typename T>
+  struct A0<T*>::B2 : A0 { };
+
+  template<typename T>
+  template<typename U>
+  struct A0<T*>::B3 : A0 { };
+}
diff --git a/clang/test/SemaTemplate/dependent-names.cpp b/clang/test/SemaTemplate/dependent-names.cpp
index 58d3d3eb4a6ff..f3a7b0915b7b1 100644
--- a/clang/test/SemaTemplate/dependent-names.cpp
+++ b/clang/test/SemaTemplate/dependent-names.cpp
@@ -336,6 +336,7 @@ template < unsigned > struct X {
   static const unsigned dimension = 3;
   template<unsigned dim=dimension>
   struct Y: Y<dim> { }; // expected-error{{base class has incomplete type}}
+                        // expected-note at -1{{definition of 'Y<dim>' is not complete until the closing '}'}}
 };
 typedef X<3> X3;
 X3::Y<>::iterator it; // expected-error {{no type named 'iterator' in 'PR11421::X<3>::Y<>'}}

>From 7ff4a0ba4ad3506e4ab70178398bdac838b91c34 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Fri, 17 May 2024 15:51:44 -0400
Subject: [PATCH 4/5] [Clang][Sema] Do not diagnose circular inheritence with
 base classes that are members of the current instantiation

---
 .../clang/Basic/DiagnosticSemaKinds.td        |  2 -
 clang/lib/Sema/SemaDeclCXX.cpp                | 49 -------------------
 .../class.derived.general/p2.cpp              | 34 ++++++++++++-
 clang/test/SemaTemplate/dependent-names.cpp   |  4 +-
 4 files changed, 35 insertions(+), 54 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index e3b4186f1b06f..e3c65cba4886a 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9464,8 +9464,6 @@ def err_static_data_member_not_allowed_in_local_class : Error<
 def err_base_clause_on_union : Error<"unions cannot have base classes">;
 def err_base_must_be_class : Error<"base specifier must name a class">;
 def err_union_as_base_class : Error<"unions cannot be base classes">;
-def err_circular_inheritance : Error<
-  "circular inheritance between %0 and %1">;
 def err_base_class_has_flexible_array_member : Error<
   "base class %0 has a flexible array member">;
 def err_incomplete_base_class : Error<"base class has incomplete type">;
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index f98339fa5748d..c92f5f5406fec 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -2656,38 +2656,6 @@ bool Sema::isCurrentClassNameTypo(IdentifierInfo *&II, const CXXScopeSpec *SS) {
   return false;
 }
 
-/// Determine whether the given class is a base class of the given
-/// class, including looking at dependent bases.
-static bool findCircularInheritance(const CXXRecordDecl *Class,
-                                    const CXXRecordDecl *Current) {
-  SmallVector<const CXXRecordDecl*, 8> Queue;
-
-  Class = Class->getCanonicalDecl();
-  while (true) {
-    for (const auto &I : Current->bases()) {
-      CXXRecordDecl *Base = I.getType()->getAsCXXRecordDecl();
-      if (!Base)
-        continue;
-
-      Base = Base->getDefinition();
-      if (!Base)
-        continue;
-
-      if (Base->getCanonicalDecl() == Class)
-        return true;
-
-      Queue.push_back(Base);
-    }
-
-    if (Queue.empty())
-      return false;
-
-    Current = Queue.pop_back_val();
-  }
-
-  return false;
-}
-
 /// Check the validity of a C++ base class specifier.
 ///
 /// \returns a new CXXBaseSpecifier if well-formed, emits diagnostics
@@ -2748,23 +2716,6 @@ Sema::CheckBaseSpecifier(CXXRecordDecl *Class,
       return nullptr;
     }
   } else if (BaseType->isDependentType()) {
-    // Make sure that we don't have circular inheritance among our dependent
-    // bases. For non-dependent bases, the check for completeness below handles
-    // this.
-    if (CXXRecordDecl *BaseDecl = BaseType->getAsCXXRecordDecl()) {
-      if (BaseDecl->getCanonicalDecl() == Class->getCanonicalDecl() ||
-          ((BaseDecl = BaseDecl->getDefinition()) &&
-           findCircularInheritance(Class, BaseDecl))) {
-        Diag(BaseLoc, diag::err_circular_inheritance)
-          << BaseType << Context.getTypeDeclType(Class);
-
-        if (BaseDecl->getCanonicalDecl() != Class->getCanonicalDecl())
-          Diag(BaseDecl->getLocation(), diag::note_previous_decl)
-            << BaseType;
-
-        return nullptr;
-      }
-    }
     // Make sure that we don't make an ill-formed AST where the type of the
     // Class is non-dependent and its attached base class specifier is an
     // dependent type, which violates invariants in many clang code paths (e.g.
diff --git a/clang/test/CXX/class.derived/class.derived.general/p2.cpp b/clang/test/CXX/class.derived/class.derived.general/p2.cpp
index 0930375a45737..888d9cd7a939d 100644
--- a/clang/test/CXX/class.derived/class.derived.general/p2.cpp
+++ b/clang/test/CXX/class.derived/class.derived.general/p2.cpp
@@ -81,4 +81,36 @@ namespace CurrentInstantiation {
   template<typename T>
   template<typename U>
   struct A0<T*>::B3 : A0 { };
-}
+} // namespace CurrentInstantiation
+
+namespace MemberOfCurrentInstantiation {
+  template<typename T>
+  struct A0 {
+    struct B : B { }; // expected-error {{base class has incomplete type}}
+                      // expected-note at -1 {{definition of 'MemberOfCurrentInstantiation::A0::B' is not complete until the closing '}'}}
+
+    template<typename U>
+    struct C : C<U> { }; // expected-error {{base class has incomplete type}}
+                         // expected-note at -1 {{definition of 'C<U>' is not complete until the closing '}'}}
+  };
+
+  template<typename T>
+  struct A1 {
+    struct B; // expected-note {{definition of 'MemberOfCurrentInstantiation::A1<long>::B' is not complete until the closing '}'}}
+
+    struct C : B { }; // expected-error {{base class has incomplete type}}
+
+    struct B : C { }; // expected-note {{in instantiation of member class 'MemberOfCurrentInstantiation::A1<long>::C' requested here}}
+  };
+
+  template struct A1<long>; // expected-note {{in instantiation of member class 'MemberOfCurrentInstantiation::A1<long>::B' requested here}}
+
+  template<>
+  struct A1<short>::B {
+    static constexpr bool f() {
+      return true;
+    }
+  };
+
+  static_assert(A1<short>::C::f());
+} // namespace MemberOfCurrentInstantiation
diff --git a/clang/test/SemaTemplate/dependent-names.cpp b/clang/test/SemaTemplate/dependent-names.cpp
index f3a7b0915b7b1..a7260b194462c 100644
--- a/clang/test/SemaTemplate/dependent-names.cpp
+++ b/clang/test/SemaTemplate/dependent-names.cpp
@@ -350,7 +350,7 @@ namespace rdar12629723 {
 
     struct B;
 
-    struct A : public B {  // expected-note{{'A' declared here}}
+    struct A : public B {
       virtual void foo() { }
     };
 
@@ -359,7 +359,7 @@ namespace rdar12629723 {
   };
 
   template<class T>
-  struct X<T>::B : public A {  // expected-error{{circular inheritance between 'A' and 'rdar12629723::X::B'}}
+  struct X<T>::B : public A {
     virtual void foo() { }
   };
 }

>From 5d903b3df21178d48fb214847449c9476f7ad197 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Fri, 17 May 2024 17:36:47 -0400
Subject: [PATCH 5/5] [FOLD] update clang-tidy test

---
 .../cppcoreguidelines/pro-type-member-init-no-crash.cpp   | 8 ++++++++
 .../checkers/cppcoreguidelines/pro-type-member-init.cpp   | 6 ------
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init-no-crash.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init-no-crash.cpp
index 300fff6cb179b..2e2964dda1daf 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init-no-crash.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init-no-crash.cpp
@@ -5,3 +5,11 @@ struct X {
   // CHECK-MESSAGES: :[[@LINE-1]]:5: error: field has incomplete type 'X' [clang-diagnostic-error]
   int a = 10;
 };
+
+template <typename T> class NoCrash {
+  // CHECK-MESSAGES: :[[@LINE+2]]:20: error: base class has incomplete type
+  // CHECK-MESSAGES: :[[@LINE-2]]:29: note: definition of 'NoCrash<T>' is not complete until the closing '}'
+  class B : public NoCrash {
+    template <typename U> B(U u) {}
+  };
+};
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp
index 8d6992afef08a..eaa73b906ce09 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp
@@ -463,12 +463,6 @@ struct NegativeIncompleteArrayMember {
   char e[];
 };
 
-template <typename T> class NoCrash {
-  class B : public NoCrash {
-    template <typename U> B(U u) {}
-  };
-};
-
 struct PositiveBitfieldMember {
   PositiveBitfieldMember() {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: F



More information about the cfe-commits mailing list