No longer try to instantiate an uninstantiated template class

when it's being typedefed.  This means we may not correctly
identify iwyu violations that we (the typedef author) are
responsible for if the typedef is never used anywhere.  On the
plus side, we'll no longer get spurious warnings about methods
that couldn't be instantiated.  And we're no longer playing
fast and loose with the AST, by modifying it after it's been
created, which has always been dangerous.

In practice, things should work fine since we pretty much
always run iwyu a package at a time.  As long as *some* .cc
file in the package where the typedef is defined, actually
tries to use the typedef, then iwyu will notice.

The proximate cause of this change is that a blaze build of
speech/trainer/mr_util:mr_decode_net would get into an
infinite loop, presumably because the AST got all confused
(due to a recent change in clang, perhaps, that made our
post-hoc modifications no longer safe).

R=wan,dsturtevant
DELTA=90  (8 added, 80 deleted, 2 changed)


Revision created by MOE tool push_codebase.
MOE_MIGRATION=1340
This commit is contained in:
csilvers+iwyu 2011-04-12 04:57:10 +00:00
parent de5266f972
commit 7ee2f365aa
3 changed files with 10 additions and 82 deletions

83
iwyu.cc
View File

@ -288,12 +288,7 @@ string IntToString(int i) {
// initializers were explicitly written, all default constructors
// were explicitly written, etc, even if they're not. We traverse
// the implicit stuff as if it were explicit.
// 5) Make sure templates in typedefs are instantiated. When we see
// a typedef, we want to simulate creating an instance of the
// underlying type. If the underlying type is never actually used,
// though, and it's a template, clang may never instantiate it.
// So we have to.
// 6) Add two callbacks that subclasses can override (just like any
// 5) Add two callbacks that subclasses can override (just like any
// other AST callback): TraverseImplicitDestructorCall and
// HandleFunctionCall. TraverseImplicitDestructorCall is a
// callback for a "pseudo-AST" node that covers destruction not
@ -309,12 +304,6 @@ string IntToString(int i) {
// visitors. Subclasses should override these Visit routines, and not
// the Traverse routine directly.
// Holds all decls (either a class or class template) where we've
// chosen to explicitly instantiate the methods of this decl. This
// makes sure we don't try to instantiate twice. Required by (5).
// TODO(csilvers): should clear this when (if) the AST ever changes.
static set<const Decl*> g_explicitly_instantiated_classes;
template <class Derived>
class BaseAstVisitor : public RecursiveASTVisitor<Derived> {
public:
@ -731,72 +720,7 @@ class BaseAstVisitor : public RecursiveASTVisitor<Derived> {
}
//------------------------------------------------------------
// (5) Make sure templates in typedefs are instantiated.
// If you do 'typedef MyClass<Foo> Bar', we basically instantiate
// MyClass<Foo> right there, and report any iwyu violations we see.
// This is because the typedef causes us to 're-export'
// MyClass<Foo>: that is, when you write 'typedef Foo Bar', clients
// can use Bar however they want without having to worry about
// #including anything except you. That puts you on the hook for
// all the #includes that Bar might need, for *anything* one might
// want to do to a Bar (basically, instantiate it or access methods
// of it).
//
// But if MyClass<Foo> is never actually used in the program, then
// clang won't bother to create an implicit instantiation of
// MyClass<Foo>. To protect against that, we fake an explicit
// instantiation at the spot of the typedef, by telling clang that
// in the code 'typedef MyClass<Foo> MyTypedef;', the 'MyClass<Foo>'
// is actually an explicit instantiation.
bool TraverseTypedefDecl(clang::TypedefDecl* decl) {
if (CanIgnoreCurrentASTNode())
return Base::TraverseTypedefDecl(decl);
const Type* underlying_type = decl->getUnderlyingType().getTypePtr();
const Decl* underlying_decl = TypeToDeclAsWritten(underlying_type);
if (const ClassTemplateSpecializationDecl* specialization_decl =
DynCastFrom(underlying_decl)) {
const TemplateSpecializationKind spec_kind
= specialization_decl->getTemplateSpecializationKind();
// TSK_Undeclared means that clang didn't see the need to
// instantiate it. Note: this call modifies specialization_decl
// in place. It does not cause specialization_decl to be
// inserted into the AST; it's only accessible via this typedef.
if (spec_kind == clang::TSK_Undeclared) {
compiler_->getSema().InstantiateClassTemplateSpecialization(
CurrentLoc(),
const_cast<ClassTemplateSpecializationDecl*>(specialization_decl),
clang::TSK_ExplicitInstantiationDefinition,
false); // no complaining!
}
// If there was already an explicit instantiation (written
// directly in the code), all members were instantiated then.
// If not -- either spec_kind was TSK_Undeclared, or was an
// implicit instantiation -- we need to make sure all members
// are instantiated. However, make sure we only ever do it once!
if (spec_kind != clang::TSK_ExplicitInstantiationDeclaration &&
spec_kind != clang::TSK_ExplicitInstantiationDefinition &&
spec_kind != clang::TSK_ExplicitSpecialization &&
!Contains(g_explicitly_instantiated_classes, specialization_decl)) {
// TODO(csilvers): this can emit warnings in badinc.cc. Figure out why.
compiler_->getSema().InstantiateClassTemplateSpecializationMembers(
CurrentLoc(),
const_cast<ClassTemplateSpecializationDecl*>(specialization_decl),
clang::TSK_ExplicitInstantiationDefinition);
g_explicitly_instantiated_classes.insert(specialization_decl);
}
}
// We want to do all this instantiation before traversing the
// typedef decl, since we have some VisitTypedefDecl() calls
// (below) which look at specialization_decl, and they need to see
// an instantiated decl. That's why we do this parent-call last.
return Base::TraverseTypedefDecl(decl);
}
//------------------------------------------------------------
// (6) Add TraverseImplicitDestructorCall and HandleFunctionCall.
// (5) Add TraverseImplicitDestructorCall and HandleFunctionCall.
// TraverseImplicitDestructorCall: This is a callback this class
// introduces that is a first-class callback just like any AST-node
@ -3262,9 +3186,6 @@ class IwyuAction : public ASTFrontendAction {
InitGlobals(&compiler.getSourceManager(),
&compiler.getPreprocessor().getHeaderSearchInfo());
// Also init the globals that are local to this file.
g_explicitly_instantiated_classes.clear();
IwyuPreprocessorInfo* const preprocessor_consumer
= new IwyuPreprocessorInfo();
VisitorState* const visitor_state

View File

@ -206,6 +206,9 @@ typedef I1_Class Cc_typedef_array[kI1ConstInt];
// IWYU: I2_Class is...*badinc-i2.h
// IWYU: I2_Class::~I2_Class is...*badinc-i2-inl.h
typedef I1_TemplateClass<I1_TemplateClass<I1_Class,I2_Class> > Cc_tpl_typedef;
// TODO(csilvers): it would be nice to be able to take this line out and
// still have the above tests pass:
Cc_tpl_typedef cc_tpl_typedef;
// IWYU: I2_Class is...*badinc-i2.h
// IWYU: I2_Class::I2_Class is...*badinc-i2-inl.h
// IWYU: I2_Class::~I2_Class is...*badinc-i2-inl.h
@ -589,6 +592,10 @@ int cc_ptr_check3 = Cc_PTS<int I1_Class::*>::IsPointer;
// badinc.h
H_Typedef h_typedef;
// TODO(csilvers): it would be nice to be able to remove this and
// still have all tests pass. As it is, we need this to instantiate
// the template's underlying type, which we need to find its violations.
H_I1_Class_Typedef h_i1_class_typedef;
H_I2Enum_Set h_i2enum_set;
H_I2Enum_Set* h_i2enum_set_ptr;
H_Class h_class;

View File

@ -291,10 +291,10 @@ typedef I2_EnumForTypedefs H_Typedef;
typedef std::set<I2_Enum> H_I2Enum_Set;
// We need the full definition of I2_Class because as a typedef we are
// re-exporting the vector<I2_Class> type, so it must be fully defined.
// TODO(csilvers): IWYU: I2_Class::~I2_Class is...*badinc-i2-inl.h
// IWYU: std::vector is...*<vector>
// IWYU: I2_Class needs a declaration
// IWYU: I2_Class is...*badinc-i2.h
// IWYU: I2_Class::~I2_Class is...*badinc-i2-inl.h
typedef std::vector<I2_Class> H_I2Class_Vector_Unused;
// IWYU: I2_TemplateClass is...*badinc-i2.h
// IWYU: I2_TemplateClass::I2_TemplateClass<.*> is...*badinc-i2-inl.h