Serene
/
include-what-you-use
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

It turns out the problem with failing tests wasn't due to 

handling of elaborated types at all, but instead was because
RecursiveASTVisitor changed from calling
TraverseNestedNameSpecifier to TraverseNestedNameSpecifierLoc
for most nns's.  We didn't subclass
TraverseNestedNameSpecifierLoc so we were missing a lot of
NNS's.

This CL backs out the previous change, and replaces it with code to
intercept and handle NNSLoc's.  The end result is the same --
badinc.cc is down to two failing tests (which are perhaps
unrelated to the clang upgrade, but instead due to changes to
iwyu itself).

I also fixed up the last two remaining badinc failures.
They were both caused by a type that I expected to be a
TemplateSpecializationType, but was actually an
ElaboratedType, because it was std::vector<...>.  (So maybe
there *was* a change wrt elaborated types in the clang code.)
I audited iwyu_ast_util.h to make sure I was putting
RemoveElaboration() calls in all the necessary places.
Hopefully I got them all...

In tracking this down, I found another bug in the same area
(nested template specializations), but I'll deal with that in
a different CL.

R=dsturtevant
DELTA=132  (75 added, 43 deleted, 14 changed)


Revision created by MOE tool push_codebase.
MOE_MIGRATION=895
This commit is contained in:
csilvers+iwyu 2011-03-18 02:53:09 +00:00
parent 7e94050119
commit 55552e6cbe
4 changed files with 89 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
iwyu.cc
View File

@ -175,6 +175,7 @@ using clang::MacroInfo;
using clang::MemberExpr;
using clang::NamedDecl;
using clang::NestedNameSpecifier;
using clang::NestedNameSpecifierLoc;
using clang::OverloadExpr;
using clang::PointerType;
using clang::PPCallbacks;
@ -413,6 +414,18 @@ class BaseAstVisitor : public RecursiveASTVisitor<Derived> {
return Base::TraverseNestedNameSpecifier(nns);
}
bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc nns_loc) {
if (!nns_loc) // using NNSLoc::operator bool()
return true;
ASTNode node(&nns_loc, *GlobalSourceManager());
CurrentASTNodeUpdater canu(&current_ast_node_, &node);
// TODO(csilvers): have VisitNestedNameSpecifierLoc instead.
if (!this->getDerived().VisitNestedNameSpecifier(
nns_loc.getNestedNameSpecifier()))
return false;
return Base::TraverseNestedNameSpecifierLoc(nns_loc);
}
bool TraverseTemplateName(TemplateName template_name) {
ASTNode node(&template_name, *GlobalSourceManager());
CurrentASTNodeUpdater canu(&current_ast_node_, &node);
@ -871,9 +884,10 @@ class BaseAstVisitor : public RecursiveASTVisitor<Derived> {
// we still want to have a parent_type, as if we were defined as
// MyClass::operator==. So we go through the arguments and take the
// first one that's a class, and associate the function with that.
if (!parent_type && GetFirstClassArgument(expr))
parent_type = GetTypeOf(GetFirstClassArgument(expr));
if (!parent_type) {
if (const Expr* first_argument = GetFirstClassArgument(expr))
parent_type = GetTypeOf(first_argument);
}
return this->getDerived().HandleFunctionCall(expr->getDirectCallee(),
parent_type);
}
@ -1029,6 +1043,9 @@ class AstFlattenerVisitor : public BaseAstVisitor<AstFlattenerVisitor> {
bool Contains(const ASTNode& node) const {
if (const TypeLoc* tl = node.GetAs<TypeLoc>()) {
return find(typelocs.begin(), typelocs.end(), *tl) != typelocs.end();
} else if (const NestedNameSpecifierLoc* nl
= node.GetAs<NestedNameSpecifierLoc>()) {
return find(nnslocs.begin(), nnslocs.end(), *nl) != nnslocs.end();
} else if (const TemplateName* tn = node.GetAs<TemplateName>()) {
// The best we can do is to compare the associated decl
if (tn->getAsTemplateDecl() == NULL)
@ -1054,6 +1071,7 @@ class AstFlattenerVisitor : public BaseAstVisitor<AstFlattenerVisitor> {
void AddAll(const NodeSet& that) {
Extend(&typelocs, that.typelocs);
Extend(&nnslocs, that.nnslocs);
Extend(&tpl_names, that.tpl_names);
Extend(&tpl_args, that.tpl_args);
Extend(&tpl_arglocs, that.tpl_arglocs);
@ -1062,11 +1080,13 @@ class AstFlattenerVisitor : public BaseAstVisitor<AstFlattenerVisitor> {
// Needed since we're treated like an stl-like object.
bool empty() const {
return (typelocs.empty() && tpl_names.empty() && tpl_args.empty() &&
return (typelocs.empty() && nnslocs.empty() &&
tpl_names.empty() && tpl_args.empty() &&
tpl_arglocs.empty() && others.empty());
}
void clear() {
typelocs.clear();
nnslocs.clear();
tpl_names.clear();
tpl_args.clear();
tpl_arglocs.clear();
@ -1078,12 +1098,14 @@ class AstFlattenerVisitor : public BaseAstVisitor<AstFlattenerVisitor> {
// It's ok not to check for duplicates; we're just traversing the tree.
void Add(TypeLoc tl) { typelocs.push_back(tl); }
void Add(NestedNameSpecifierLoc nl) { nnslocs.push_back(nl); }
void Add(TemplateName tn) { tpl_names.push_back(tn); }
void Add(TemplateArgument ta) { tpl_args.push_back(ta); }
void Add(TemplateArgumentLoc tal) { tpl_arglocs.push_back(tal); }
void Add(const void* o) { others.insert(o); }
vector<TypeLoc> typelocs;
vector<NestedNameSpecifierLoc> nnslocs;
vector<TemplateName> tpl_names;
vector<TemplateArgument> tpl_args;
vector<TemplateArgumentLoc> tpl_arglocs;
@ -1451,9 +1473,6 @@ class IwyuBaseAstVisitor : public BaseAstVisitor<Derived> {
const Type* underlying_type = decl->getUnderlyingType().getTypePtr();
const NamedDecl* underlying_decl = TypeToDeclAsWritten(underlying_type);
if (underlying_decl && isa<TypedefDecl>(underlying_decl))
return true; // we always re-export a chained typedef
if (underlying_decl && GetDefinitionForClass(underlying_decl) == NULL)
return false; // case (1)
@ -2361,6 +2380,9 @@ class InstantiatedTemplateVisitor
}
// TODO(csilvers): have TraverseSubstTemplateTypeParmType that
// Traverses actual_type.
// These do the actual work of finding the types to return. Our
// task is made easier since (at least in theory), every time we
// instantiate a template type, the instantiation has type
@ -2380,7 +2402,7 @@ class InstantiatedTemplateVisitor
// TODO(csilvers): consider encoding this logic via
// in_forward_declare_context. I think this will require changing
// in_forward_declare_context to yes/no/maybe.
if (IsClassElaborationQualifier(current_ast_node())) {
if (current_ast_node()->ParentIsA<NestedNameSpecifier>()) {
ReportTypesUse(CurrentLoc(), types_of_interest);
return Base::VisitSubstTemplateTypeParmType(type);
}
@ -3018,15 +3040,11 @@ class IwyuAstConsumer
bool VisitTagType(clang::TagType* type) {
if (CanIgnoreCurrentASTNode()) return true;
// If we're a class elaboration, like Foo in Foo::FooSubclass, we
// need a full definition, even if we're in a forward-declare
// context. Otherwise, if we're forward-declarable, then no
// complicated checking is needed: just forward-declare. If we're
// already elaborated ('class Foo x') but not namespace-qualified
// ('class ns::Foo x') there's no need even to forward-declare!
if (IsClassElaborationQualifier(current_ast_node())) {
current_ast_node()->set_in_forward_declare_context(false);
} else if (CanForwardDeclareType(current_ast_node())) {
// If we're forward-declarable, then no complicated checking is
// needed: just forward-declare. If we're already elaborated
// ('class Foo x') but not namespace-qualified ('class ns::Foo x')
// there's no need even to forward-declare!
if (CanForwardDeclareType(current_ast_node())) {
current_ast_node()->set_in_forward_declare_context(true);
if (!IsElaborationNode(current_ast_node()->parent()) ||
IsNamespaceQualifiedNode(current_ast_node()->parent())) {

81
iwyu_ast_util.h
View File

@ -73,6 +73,10 @@ class ASTNode {
ASTNode(const clang::NestedNameSpecifier* nns, const clang::SourceManager& sm)
: kind_(kNNSKind), as_nns_(nns),
parent_(NULL), in_fwd_decl_context_(false), source_manager_(sm) { }
ASTNode(const clang::NestedNameSpecifierLoc* nnsloc,
const clang::SourceManager& sm)
: kind_(kNNSLocKind), as_nnsloc_(nnsloc),
parent_(NULL), in_fwd_decl_context_(false), source_manager_(sm) { }
ASTNode(const clang::TemplateName* template_name,
const clang::SourceManager& sm)
: kind_(kTemplateNameKind), as_template_name_(template_name),
@ -233,7 +237,7 @@ class ASTNode {
private:
enum NodeKind {
kDeclKind, kStmtKind, kTypeKind, kTypelocKind, kNNSKind,
kDeclKind, kStmtKind, kTypeKind, kTypelocKind, kNNSKind, kNNSLocKind,
kTemplateNameKind, kTemplateArgumentKind, kTemplateArgumentLocKind
};
@ -269,9 +273,18 @@ class ASTNode {
}
template<typename To> const To* DynCast(
const clang::NestedNameSpecifier*) const {
// Like Type, this will cast to NNS if we're an NNSLoc. For code
// that cares to distinguish, it should check for nnslocs first.
if (kind_ == kNNSLocKind)
return as_nnsloc_->getNestedNameSpecifier();
if (kind_ != kNNSKind) return NULL;
return as_nns_;
}
template<typename To> const To* DynCast(
const clang::NestedNameSpecifierLoc*) const {
if (kind_ != kNNSLocKind) return NULL;
return as_nnsloc_;
}
template<typename To> const To* DynCast(const clang::TemplateName*) const {
if (kind_ != kTemplateNameKind) return NULL;
return as_template_name_;
@ -300,6 +313,7 @@ class ASTNode {
case kTypeKind: return as_type_;
case kTypelocKind: return as_typeloc_;
case kNNSKind: return as_nns_;
case kNNSLocKind: return as_nnsloc_;
case kTemplateNameKind: return as_template_name_;
case kTemplateArgumentKind: return as_template_arg_;
case kTemplateArgumentLocKind: return as_template_argloc_;
@ -321,6 +335,9 @@ class ASTNode {
case kTypelocKind:
*loc = GetLocation(as_typeloc_);
return true;
case kNNSLocKind:
*loc = GetLocation(as_nnsloc_);
return true;
case kTemplateArgumentLocKind:
*loc = GetLocation(as_template_argloc_);
return true;
@ -342,6 +359,7 @@ class ASTNode {
const clang::Type* as_type_;
const clang::TypeLoc* as_typeloc_;
const clang::NestedNameSpecifier* as_nns_;
const clang::NestedNameSpecifierLoc* as_nnsloc_;
const clang::TemplateName* as_template_name_;
const clang::TemplateArgument* as_template_arg_;
const clang::TemplateArgumentLoc* as_template_argloc_;
@ -415,34 +433,6 @@ inline bool IsNamespaceQualifiedNode(const ASTNode* ast_node) {
clang::NestedNameSpecifier::Namespace);
}
// See if a given ast_node is part of a class elaboration: that
// is, 'Foo' in 'Foo::FooSubclass'. The actual input should be
// a RecordType, that might or might not be to the left of a ::.
inline bool IsClassElaborationQualifier(const ASTNode* ast_node) {
if (ast_node == NULL) return false;
const clang::Type* node_type = ast_node->GetAs<clang::Type>();
if (!node_type) return false;
// Work properly even when we're a template parameter.
if (const clang::SubstTemplateTypeParmType* subst
= dyn_cast<clang::SubstTemplateTypeParmType>(node_type))
node_type = subst->getReplacementType().getTypePtr();
const clang::RecordType* record_type = dyn_cast<clang::RecordType>(node_type);
if (!record_type) return false;
const clang::ElaboratedType* elaborated_type =
ast_node->GetParentAs<clang::ElaboratedType>();
if (!elaborated_type) return false;
const clang::NestedNameSpecifier* elab_nns = elaborated_type->getQualifier();
if (!elab_nns) return false;
// Is the type in the elaboration-field us? If so, we're the elaboration.
return (elab_nns->getKind() == clang::NestedNameSpecifier::TypeSpec &&
elab_nns->getAsType() == ast_node->GetAs<clang::Type>());
}
// See if the given ast_node is the decl of a class or function that
// occurs in a friend context. That is, for 'friend class Foo', this
// function matches the decl for 'class Foo'. (It does not match the
@ -780,9 +770,13 @@ inline void AddTypelikeTemplateArgTo(const clang::TemplateArgument& tpl_arg,
VERRS(6) << "Adding a template type of interest: "
<< PrintableType(*it) << "\n";
argset->insert(*it);
// Recurse if we ourself are a template type.
// Recurse if we ourself are a template type. Read through elaborations.
const clang::Type* subtype = *it;
// TODO(csilvers): use RemoveElaboration() instead.
if (const clang::ElaboratedType* elab_type = DynCastFrom(subtype))
subtype = elab_type->getNamedType().getTypePtr();
if (const clang::TemplateSpecializationType* tpl_type
= DynCastFrom(*it)) {
= DynCastFrom(subtype)) {
for (unsigned i = 0; i < tpl_type->getNumArgs(); ++i)
AddTypelikeTemplateArgTo(tpl_type->getArg(i), argset);
}
@ -863,6 +857,13 @@ inline set<const clang::Type*> GetTplTypeArgsOfFunction(
for (unsigned i = 0; i < tpl_list->size(); ++i) {
AddTypelikeTemplateArgTo(tpl_list->get(i), &retval);
}
// TODO(csilvers): for derived tpl args (Foo(arg), not
// Foo<type>(arg)), remove types from retval if they don't appear
// somewhere in a function arg (or return value) as typed. This
// is a simple heuristic for dealing with typedefs and default
// arguments. For instance, cout << "a" should not yield
// char_traits<char> here, since the first argument of operator<<
// is typed ostream, not basic_ostream<char, char_traits<char> >.
}
return retval;
}
@ -1065,11 +1066,6 @@ inline const clang::Type* GetTypeOf(const clang::TypeDecl* decl) {
}
// Returns true if the type has any template arguments.
inline bool IsTemplatizedType(const clang::Type* type) {
return type && isa<clang::TemplateSpecializationType>(type);
}
// The ElaborationType -- which says whether a type is preceded by
// 'class' or 'struct' ('class Foo'), or whether the type-name has a
// namespace ('ns::Foo') -- often pops where it's not wanted. This
@ -1083,6 +1079,12 @@ inline const clang::Type* RemoveElaboration(const clang::Type* type) {
return type;
}
// Returns true if the type has any template arguments.
inline bool IsTemplatizedType(const clang::Type* type) {
return (type &&
isa<clang::TemplateSpecializationType>(RemoveElaboration(type)));
}
// Read past SubstTemplateTypeParmType to the underlying type, if type
// is itself a SubstTemplateTypeParmType. Thus: T is converted to int
// if we are parsing a template instantiated with T being int.
@ -1235,6 +1237,7 @@ inline bool CanImplicitlyConvertTo(const clang::Type* type) {
inline set<const clang::Type*> GetExplicitTplTypeArgsOf(
const clang::Type* type) {
set<const clang::Type*> retval;
type = RemoveElaboration(type); // get rid of the class keyword
const clang::TemplateSpecializationType* tpl_spec_type = DynCastFrom(type);
if (!tpl_spec_type)
return retval;
@ -1287,11 +1290,15 @@ inline const clang::Expr* GetFirstClassArgument(clang::CallExpr* expr) {
for (clang::CallExpr::arg_iterator it = expr->arg_begin();
it != expr->arg_end(); ++it) {
const clang::Type* argtype = GetTypeOf(*it);
// Luckily, this code does the right thing given a function like
// Make sure we do the right thing given a function like
// template <typename T> void operator>>(const T& x, ostream& os);
// In this case ('myclass >> os'), we want to be returning the
// type of os, not of myclass, and we do, because myclass will be
// a SubstTemplateTypeParmType, not a RecordType.
if (isa<clang::SubstTemplateTypeParmType>(argtype))
continue;
// TODO(csilvers): uncomment this and fix up tests to match.
//argtype = argtype->getUnqualifiedDesugaredType(); // see through typedefs
if (isa<clang::RecordType>(argtype) ||
isa<clang::TemplateSpecializationType>(argtype)) {
return *it;

5
iwyu_location_util.cc
View File

@ -104,6 +104,11 @@ clang::SourceLocation GetLocation(const clang::TypeLoc* typeloc) {
return typeloc->getBeginLoc();
}
clang::SourceLocation GetLocation(const clang::NestedNameSpecifierLoc* nnsloc) {
if (nnsloc == NULL) return clang::SourceLocation();
return nnsloc->getBeginLoc();
}
clang::SourceLocation GetLocation(const clang::TemplateArgumentLoc* argloc) {
if (argloc == NULL) return clang::SourceLocation();
return argloc->getLocation();

8
iwyu_location_util.h
View File

@ -44,12 +44,13 @@
#include <string>
#include "port.h"
#include "clang/AST/Decl.h"
#include "clang/Basic/FileManager.h"
#include "clang/AST/NestedNameSpecifier.h"
#include "clang/AST/Stmt.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/AST/TemplateBase.h"
#include "clang/Lex/Token.h"
#include "clang/AST/TypeLoc.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Lex/Token.h"
#include "iwyu_globals.h"
#include "iwyu_path_util.h"
@ -178,6 +179,7 @@ inline clang::SourceLocation GetLocation(const clang::Token& token) {
clang::SourceLocation GetLocation(const clang::Decl* token);
clang::SourceLocation GetLocation(const clang::Stmt* stmt);
clang::SourceLocation GetLocation(const clang::TypeLoc* typeloc);
clang::SourceLocation GetLocation(const clang::NestedNameSpecifierLoc* nnsloc);
clang::SourceLocation GetLocation(const clang::TemplateArgumentLoc* argloc);
// These define default implementations of GetFileEntry() and