[cfe-commits] [patch] Fix PR6353 and some related issues
Rafael Espindola
espindola at google.com
Mon Mar 1 20:56:13 PST 2010
>> One part of the patch I am not very happy with is the change to
>> SemaExpr. It is there to match the logic in EmitBaseInitializer. The
>> problem is that it is always marking the destructor as used. We should
>> do so only if it is actually being used in a base initializer and
>> exceptions are enabled. Suggestions on how to do so are welcome.
>
>
> Sema::BuildBaseInitializer is where we perform type-checking for base initializers; I suggest putting this logic there. I also feel like we should always mark the destructor (even when exceptions are disabled), because -fno-exceptions should not change the behavior of the front end for non-exception constructs.
The problem with Sema::BuildBaseInitializer is that it is not used in
implicit cases like
------------------------------------------
struct TypedInit {
virtual ~TypedInit();
};
struct OpInit : public TypedInit {
virtual void resolveBitReference();
};
struct BinOpInit : public OpInit {
virtual void getAsString() const;
};
void foobar() {
new BinOpInit();
}
---------------------------------------------
The new patch is less aggressive then the old one. Now when marking a
constructor as referenced we walk the initializers and mark the
destructors for those classes as referenced.
I will try to go over the rest of the review tomorrow.
Cheers,
--
Rafael Ávila de Espíndola
-------------- next part --------------
diff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp
index 5236619..91c7322 100644
--- a/lib/CodeGen/CodeGenModule.cpp
+++ b/lib/CodeGen/CodeGenModule.cpp
@@ -752,14 +752,20 @@ llvm::Constant *CodeGenModule::GetOrCreateLLVMFunction(const char *MangledName,
// A called constructor which has no definition or declaration need be
// synthesized.
else if (const CXXConstructorDecl *CD = dyn_cast<CXXConstructorDecl>(FD)) {
- if (CD->isImplicit())
+ if (CD->isImplicit()) {
+ assert (CD->isUsed());
DeferredDeclsToEmit.push_back(D);
+ }
} else if (const CXXDestructorDecl *DD = dyn_cast<CXXDestructorDecl>(FD)) {
- if (DD->isImplicit())
+ if (DD->isImplicit()) {
+ assert (DD->isUsed());
DeferredDeclsToEmit.push_back(D);
+ }
} else if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(FD)) {
- if (MD->isCopyAssignment() && MD->isImplicit())
+ if (MD->isCopyAssignment() && MD->isImplicit()) {
+ assert (MD->isUsed());
DeferredDeclsToEmit.push_back(D);
+ }
}
}
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index 20a8303..0dcd042 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -5791,55 +5791,70 @@ Sema::ActOnCXXConditionDeclaration(Scope *S, Declarator &D) {
return Dcl;
}
-void Sema::MaybeMarkVirtualMembersReferenced(SourceLocation Loc,
- CXXMethodDecl *MD) {
+static bool needsVtable(CXXMethodDecl *MD, ASTContext &Context) {
// Ignore dependent types.
if (MD->isDependentContext())
- return;
-
+ return false;
+
+ // Ignore declarations that are not definitions.
+ if (!MD->isThisDeclarationADefinition())
+ return false;
+
CXXRecordDecl *RD = MD->getParent();
-
+
// Ignore classes without a vtable.
if (!RD->isDynamicClass())
- return;
+ return false;
- // Ignore declarations that are not definitions.
- if (!MD->isThisDeclarationADefinition())
- return;
-
- if (isa<CXXConstructorDecl>(MD)) {
- switch (MD->getParent()->getTemplateSpecializationKind()) {
- case TSK_Undeclared:
- case TSK_ExplicitSpecialization:
- // Classes that aren't instantiations of templates don't need their
- // virtual methods marked until we see the definition of the key
- // function.
- return;
-
- case TSK_ImplicitInstantiation:
- case TSK_ExplicitInstantiationDeclaration:
- case TSK_ExplicitInstantiationDefinition:
- // This is a constructor of a class template; mark all of the virtual
- // members as referenced to ensure that they get instantiatied.
- break;
- }
- } else if (!MD->isOutOfLine()) {
- // Consider only out-of-line definitions of member functions. When we see
- // an inline definition, it's too early to compute the key function.
- return;
- } else if (const CXXMethodDecl *KeyFunction = Context.getKeyFunction(RD)) {
- // If this is not the key function, we don't need to mark virtual members.
- if (KeyFunction->getCanonicalDecl() != MD->getCanonicalDecl())
- return;
- } else {
- // The class has no key function, so we've already noted that we need to
- // mark the virtual members of this class.
- return;
+ switch (MD->getParent()->getTemplateSpecializationKind()) {
+ case TSK_Undeclared:
+ case TSK_ExplicitSpecialization:
+ // Classes that aren't instantiations of templates don't need their
+ // virtual methods marked until we see the definition of the key
+ // function.
+ break;
+
+ case TSK_ImplicitInstantiation:
+ // This is a constructor of a class template; mark all of the virtual
+ // members as referenced to ensure that they get instantiatied.
+ if (isa<CXXConstructorDecl>(MD))
+ return true;
+ case TSK_ExplicitInstantiationDeclaration:
+ case TSK_ExplicitInstantiationDefinition:
+ // This is method of a explicit instantiation; mark all of the virtual
+ // members as referenced to ensure that they get instantiatied.
+ return true;
}
-
+
+ // Consider only out-of-line definitions of member functions. When we see
+ // an inline definition, it's too early to compute the key function.
+ if (!MD->isOutOfLine())
+ return false;
+
+ const CXXMethodDecl *KeyFunction = Context.getKeyFunction(RD);
+
+ // If there is no key function, we will need a copy of the vtable.
+ if (!KeyFunction)
+ return true;
+
+ // If this is the key function, we need to mark virtual members.
+ if (KeyFunction->getCanonicalDecl() == MD->getCanonicalDecl())
+ return true;
+
+ return false;
+}
+
+void Sema::MaybeMarkVirtualMembersReferenced(SourceLocation Loc,
+ CXXMethodDecl *MD) {
+ CXXRecordDecl *RD = MD->getParent();
+
// We will need to mark all of the virtual members as referenced to build the
// vtable.
- ClassesWithUnmarkedVirtualMembers.push_back(std::make_pair(RD, Loc));
+ // We actually call MarkVirtualMembersReferenced instead of adding to
+ // ClassesWithUnmarkedVirtualMembers because this marking is needed by
+ // codegen that will happend before we finish parsing the file.
+ if (needsVtable(MD, Context))
+ MarkVirtualMembersReferenced(Loc, RD);
}
bool Sema::ProcessPendingClassesWithUnmarkedVirtualMembers() {
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index d9464ad..4b1db94 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -7207,7 +7207,24 @@ void Sema::MarkDeclarationReferenced(SourceLocation Loc, Decl *D) {
if (!Constructor->isUsed())
DefineImplicitCopyConstructor(Loc, Constructor, TypeQuals);
}
-
+
+ for (CXXConstructorDecl::init_const_iterator B = Constructor->init_begin(),
+ E = Constructor->init_end();
+ B != E; ++B) {
+ CXXBaseOrMemberInitializer *Member = (*B);
+ if (!Member->isBaseInitializer())
+ continue;
+
+ const Type *BaseType = Member->getBaseClass();
+ CXXRecordDecl *BaseClassDecl =
+ cast<CXXRecordDecl>(BaseType->getAs<RecordType>()->getDecl());
+ if (BaseClassDecl->hasTrivialDestructor())
+ continue;
+
+ CXXDestructorDecl *DD = BaseClassDecl->getDestructor(Context);
+ MarkDeclarationReferenced(Loc, DD);
+ }
+
MaybeMarkVirtualMembersReferenced(Loc, Constructor);
} else if (CXXDestructorDecl *Destructor = dyn_cast<CXXDestructorDecl>(D)) {
if (Destructor->isImplicit() && !Destructor->isUsed())
diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp
index 23d52ad..c8ccff2 100644
--- a/lib/Sema/SemaExprCXX.cpp
+++ b/lib/Sema/SemaExprCXX.cpp
@@ -426,6 +426,16 @@ bool Sema::CheckCXXThrowOperand(SourceLocation ThrowLoc, Expr *&E) {
: diag::err_throw_incomplete)
<< E->getSourceRange()))
return true;
+
+ const RecordType *RT = Ty->getAs<RecordType>();
+ if (!RT)
+ return false;
+
+ const CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());
+ if (RD->hasTrivialCopyConstructor())
+ return false;
+ CXXConstructorDecl *CopyCtor = RD->getCopyConstructor(Context, 0);
+ MarkDeclarationReferenced(ThrowLoc, CopyCtor);
}
// FIXME: Construct a temporary here.
diff --git a/test/SemaCXX/implicit-virtual-member-functions.cpp b/test/SemaCXX/implicit-virtual-member-functions.cpp
index 4ae9eae..1bb5adb 100644
--- a/test/SemaCXX/implicit-virtual-member-functions.cpp
+++ b/test/SemaCXX/implicit-virtual-member-functions.cpp
@@ -15,9 +15,9 @@ void B::f() { // expected-note {{implicit default destructor for 'struct B' firs
struct C : A { // expected-error {{no suitable member 'operator delete' in 'C'}}
C();
void operator delete(void *, int); // expected-note {{'operator delete' declared here}}
-}; // expected-note {{implicit default destructor for 'struct C' first required here}}
+};
-C::C() { }
+C::C() { } // expected-note {{implicit default destructor for 'struct C' first required here}}
struct D : A { // expected-error {{no suitable member 'operator delete' in 'D'}}
void operator delete(void *, int); // expected-note {{'operator delete' declared here}}
diff --git a/test/SemaTemplate/virtual-member-functions.cpp b/test/SemaTemplate/virtual-member-functions.cpp
index db24313..58ac08c 100644
--- a/test/SemaTemplate/virtual-member-functions.cpp
+++ b/test/SemaTemplate/virtual-member-functions.cpp
@@ -14,7 +14,7 @@ template<class T> int A<T>::a(T x) {
}
void f(A<int> x) {
- x.anchor();
+ x.anchor(); // expected-note{{in instantiation of member function 'PR5557::A<int>::anchor' requested here}}
}
template<typename T>
@@ -36,10 +36,10 @@ struct Base {
template<typename T>
struct Derived : Base<T> {
- virtual void foo() { }
+ virtual void foo() { } // expected-note {{in instantiation of member function 'Base<int>::~Base' requested here}}
};
-template struct Derived<int>; // expected-note{{instantiation}}
+template struct Derived<int>;
template<typename T>
struct HasOutOfLineKey {
@@ -52,4 +52,4 @@ T *HasOutOfLineKey<T>::f(float *fp) {
return fp; // expected-error{{cannot initialize return object of type 'int *' with an lvalue of type 'float *'}}
}
-HasOutOfLineKey<int> out_of_line;
+HasOutOfLineKey<int> out_of_line; // expected-note{{in instantiation of member function 'HasOutOfLineKey<int>::HasOutOfLineKey' requested here}}
More information about the cfe-commits
mailing list