llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Krystian Stasiowski (sdkrystian)
<details>
<summary>Changes</summary>
Consider the following:
```cpp
template<typename T>
struct A
{
struct B : A { };
};
```
According to [[class.derived.general]
p2](http://eel.is/c++draft/class.derived.general#<!-- -->2):
> [...] A _class-or-decltype_ shall denote a (possibly cv-qualified) class
type that is not an incompletely defined class; any cv-qualifiers are ignored.
[...]
Although GCC and EDG rejects this, Clang accepts it. This is incorrect, as `A`
is incomplete within its own definition (outside of a complete-class context).
This patch correctly diagnoses instances where the current instantiation is
used as a base class before it is complete.
Conversely, Clang erroneously rejects the following:
```cpp
template<typename T>
struct A
{
struct B;
struct C : B { };
struct B : C { }; // error: circular inheritance between 'C' and 'A::B'
};
```
Though it may seem like no valid specialization of this template can be
instantiated, an explicit specialization of either member classes for an
implicit instantiated specialization of `A` would permit the definition of the
other member class to be instantiated, e.g.:
```cpp
template<>
struct A<int>::B { };
A<int>::C c; // ok
```
So this patch also does away with this error. This means that circular
inheritance is diagnosed during instantiation of the definition as a
consequence of requiring the base class types to be complete (matching the
behavior of GCC and EDG).
---
Full diff: https://github.com/llvm/llvm-project/pull/92597.diff
8 Files Affected:
- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (-2)
- (modified) clang/lib/AST/Type.cpp (+9)
- (modified) clang/lib/Sema/SemaDeclCXX.cpp (+50-108)
- (modified)
clang/test/CXX/basic/basic.lookup/basic.lookup.qual/class.qual/p2.cpp (+7-3)
- (added) clang/test/CXX/class.derived/class.derived.general/p2.cpp (+116)
- (modified) clang/test/SemaTemplate/dependent-names.cpp (+8-6)
- (modified) clang/test/SemaTemplate/destructor-template.cpp (+8-6)
- (modified) clang/test/SemaTemplate/typo-dependent-name.cpp (+5-2)
``````````diff
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 59f44bc26eaf2..ee781d3155f66 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9449,8 +9449,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/AST/Type.cpp b/clang/lib/AST/Type.cpp
index e31741cd44240..545725d1d7511 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -2372,6 +2372,15 @@ bool Type::isIncompleteType(NamedDecl **Def) const {
*Def = Rec;
return !Rec->isCompleteDefinition();
}
+ case InjectedClassName: {
+ CXXRecordDecl *Rec = cast<InjectedClassNameType>(CanonicalType)->getDecl();
+ if (Rec->isBeingDefined()) {
+ if (Def)
+ *Def = Rec;
+ return true;
+ }
+ return false;
+ }
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..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
@@ -2704,111 +2672,79 @@ 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();
-
- 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;
+ 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;
+ }
- 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 (RequireCompleteType(BaseLoc, BaseType, diag::err_incomplete_base_class,
+ SpecifierRange)) {
+ Class->setInvalidDecl();
+ return nullptr;
+ }
+ } else if (BaseType->isDependentType()) {
// 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.
// 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()) {
+ } else {
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 +2754,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 +2822,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/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..888d9cd7a939d
--- /dev/null
+++ b/clang/test/CXX/class.derived/class.derived.general/p2.cpp
@@ -0,0 +1,116 @@
+// 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 { };
+} // namespace CurrentInstantiation
+
+namespace MemberOfCurrentInstantiation {
+ template<typename T>
+ struct A0 {
+ struct B : B { }; // expected-error {{base class has incomplete type}}
+ // expected-note@-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@-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 641ec950054f5..a7260b194462c 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,9 @@ 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}}
+ // expected-note@-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<>'}}
@@ -344,11 +345,12 @@ 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@-1{{definition of
'rdar12629723::X::C' is not complete until the closing '}'}}
struct B;
- struct A : public B { // expected-note{{'A' declared here}}
+ struct A : public B {
virtual void foo() { }
};
@@ -357,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() { }
};
}
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 88b2fc373b1f5..5e00f576a5084 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()); }
``````````
</details>
https://github.com/llvm/llvm-project/pull/92597
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
