Work in progress on safer iterator detection.
Based on our conversations, I've redone the iterator detection. This patch is
highly unpolished and I'm posting it for feedback on the algorithm. I now
detect
only standard iterator names of standard containers or more sugared types.
This
work is done all by matchers now. Some notes on the matchers:
- I'm thinking about contributing hasDeclContext, hasSpecifier, namesType to
the ASTMatchers library.
- Also included would be the 'has member getDecl' type traits test.
- hasDecl2 is similar to hasDecl except it's more generic. Is it safe to
replace
the existing one that works only on typedefs? If so, how should we
distinguish
it from hasDeclaration which has similar behaviour but looks through sugar.
- The matchers hasStd*Name() is really just to avoid needing more overloads of
allOf(). They're slightly more efficient than the alternative too.
Hi klimek,
http://llvm-reviews.chandlerc.com/D392
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D392?vs=958&id=1007#toc
Files:
cpp11-migrate/UseAuto/UseAutoActions.cpp
cpp11-migrate/UseAuto/UseAutoMatchers.cpp
Index: cpp11-migrate/UseAuto/UseAutoActions.cpp
===================================================================
--- cpp11-migrate/UseAuto/UseAutoActions.cpp
+++ cpp11-migrate/UseAuto/UseAutoActions.cpp
@@ -18,6 +18,91 @@
using namespace clang::tooling;
using namespace clang;
+namespace {
+void unwrap(QualType QT, SourceManager &SM) {
+ llvm::errs() << "Details for " << QT.getAsString() << "\n";
+ switch (QT->getTypeClass()) {
+ case Type::Elaborated: {
+ const ElaboratedType *ElabTy = cast<ElaboratedType>(QT.getTypePtr());
+ NestedNameSpecifier *Name = ElabTy->getQualifier();
+ switch (Name->getKind()) {
+ case NestedNameSpecifier::Identifier:
+ llvm::errs() << " Elaborated Id\n";
+ break;
+ case NestedNameSpecifier::Namespace: {
+ llvm::errs() << " Elaborated namespace\n";
+ NamespaceDecl *ND = Name->getAsNamespace();
+ assert(ND);
+ llvm::errs() << " " << ND->getName() << "\n";
+ llvm::errs() << " " << SM.getFileEntryForID(SM.getFileID(ND->getLocStart()))->getName() <<
+ ":" << SM.getSpellingLineNumber(ND->getLocStart()) << "\n";
+ } break;
+ case NestedNameSpecifier::NamespaceAlias:
+ llvm::errs() << " Elaborated namespace alias\n";
+ break;
+ case NestedNameSpecifier::TypeSpec: {
+ llvm::errs() << " Elaborated typespec\n";
+ const Type *Ty = Name->getAsType();
+ Ty->dump();
+ llvm::errs() << " " << Ty->getTypeClassName() << "\n";
+ const RecordType *RTy = dyn_cast<RecordType>(Ty);
+ if (RTy) {
+ llvm::errs() << " " << RTy->getDecl()->getQualifiedNameAsString() << "\n";
+ }
+ const RecordType *NamedRTy = cast<RecordType>(ElabTy->getNamedType().getTypePtr());
+ llvm::errs() << " " << NamedRTy->getDecl()->getName() << "\n";
+
+
+ } break;
+ case NestedNameSpecifier::TypeSpecWithTemplate:
+ llvm::errs() << " Elaborated Type template\n";
+ break;
+ case NestedNameSpecifier::Global:
+ llvm::errs() << " Elaborated Glob\n";
+ break;
+ }
+ } break;
+ case Type::Typedef: {
+ llvm::errs() << " Typedef\n";
+ const TypedefType *Ty = cast<TypedefType>(QT.getTypePtr());
+ TypedefNameDecl *NameDecl = Ty->getDecl();
+ llvm::errs() << " " << NameDecl->getName() << "\n";
+ RecordDecl *RD = dyn_cast<RecordDecl>(NameDecl->getDeclContext());
+ if (!RD) {
+ llvm::errs() << " decl context not a record\n";
+ break;
+ }
+ llvm::errs() << " " << RD->getQualifiedNameAsString() << "\n";
+ //RD->getTypeForDecl()->dump();
+
+ //const TypeLoc &TL = Ty->getDecl()->getTypeSourceInfo()->getTypeLoc();
+ //llvm::errs() << " " << SM.getFileEntryForID(SM.getFileID(TL.getLocStart()))->getName() <<
+ // ":" << SM.getSpellingLineNumber(TL.getLocStart()) << "\n";
+ } break;
+ default: {
+ switch (QT->getTypeClass()) {
+#define ABSTRACT_TYPE(Class, Parent)
+#define TYPE(Class, Parent) \
+ case Type::Class: \
+ llvm::errs() << " " #Class "\n"; break;
+#include "clang/AST/TypeNodes.def"
+ }
+ }
+ }
+ if (QT->isCanonicalUnqualified()) {
+ llvm::errs() << " is canonical\n";
+ }
+}
+}
+
+// Notes:
+// For Elaborated - can use built-in stuff to get namespace it seems
+// for typedef - can get decl and from there up through DeclContext to get
+// namespace (?)
+// for record - as above
+// template specialization - Don't do anything? Just unwrap another layer of
+// sugar.
+
void UseAutoFixer::run(const MatchFinder::MatchResult &Result) {
const VarDecl *D = Result.Nodes.getNodeAs<VarDecl>(DeclNodeId);
@@ -33,25 +118,43 @@
// Drill down to the as-written initializer.
const Expr *E = Construct->arg_begin()->IgnoreParenImpCasts();
+ if (E != E->IgnoreConversionOperator())
+ // We hit a conversion operator. Early-out now as they imply an implicit
+ // conversion from a different type. Could also mean an explicit conversion
+ // from the same type but that's pretty rare.
+ return;
- while (true) {
- const Expr *E2 = E->IgnoreParenImpCasts();
- if (E2 != E) {
- E = E2;
- continue;
- }
+ //llvm::errs() << "Line " << SM.getSpellingLineNumber(D->getLocStart()) << "\n";
+ //QualType DQT = D->getType();
+ //QualType IQT = E->getType();
- E2 = E->IgnoreConversionOperator();
- if (E2 != E) {
- // We hit a conversion operator. Since they may do non-trivial work,
- // making use of auto would result in this operator no-longer being
- // called.
- return;
- }
+ //bool Change = false;
+ //std::set<std::string> SeenTypes;
+ //do {
+ // std::string DQTStr = DQT.getAsString();
+ // std::string IQTStr = IQT.getAsString();
+ // llvm::errs() << DQTStr << " = " << IQTStr << "\n";
+ // if (SeenTypes.find(DQTStr) == SeenTypes.end()) {
+ // SeenTypes.insert(DQTStr);
+ // unwrap(DQT, SM);
+ // }
+ // if (SeenTypes.find(IQTStr) == SeenTypes.end()) {
+ // SeenTypes.insert(IQTStr);
+ // unwrap(IQT, SM);
+ // }
- // Nothing changed. We're down as far as we can go.
- break;
- }
+ // QualType DQT_ = DQT.getSingleStepDesugaredType(*Result.Context);
+ // QualType IQT_ = IQT.getSingleStepDesugaredType(*Result.Context);
+ // Change = false;
+ // if (DQT != DQT_) {
+ // DQT = DQT_;
+ // Change = true;
+ // }
+ // if (IQT != IQT_) {
+ // IQT = IQT_;
+ // Change = true;
+ // }
+ //} while (Change);
// Compare canonical types so typedefs don't mess the comparison up.
if (D->getType().getCanonicalType() == E->getType().getCanonicalType()) {
Index: cpp11-migrate/UseAuto/UseAutoMatchers.cpp
===================================================================
--- cpp11-migrate/UseAuto/UseAutoMatchers.cpp
+++ cpp11-migrate/UseAuto/UseAutoMatchers.cpp
@@ -57,21 +57,174 @@
return !ImplicitInit;
}
+AST_MATCHER_P(QualType, hasSugar, internal::Matcher<QualType>, SugarMatcher) {
+ QualType QT = Node;
+ //llvm::errs() << QT.getAsString() << "\n";
+ for (;;) {
+
+ if (QT->getTypeClass() == Type::Typedef) {
+ int i = 9;
+ }
+ if (SugarMatcher.matches(QT, Finder, Builder)) {
+ //llvm::errs() << " Matched!\n";
+ return true;
+ }
+
+ QualType NewQT = QT.getSingleStepDesugaredType(Finder->getASTContext());
+ if (NewQT == QT) {
+ break;
+ }
+ QT = NewQT;
+ //llvm::errs() << " " << QT.getAsString() << "\n";
+ }
+ return false;
+}
+
+AST_TYPE_MATCHER(RecordType, recordType);
+AST_TYPE_MATCHER(ElaboratedType, elaboratedType);
+
+template<typename T>
+struct has_getDecl {
+ typedef char yes[1];
+ typedef char no [2];
+ template <typename _1> static yes &chk(char[sizeof(&_1::getDecl)]);
+ template <typename > static no &chk(...);
+ static bool const value = sizeof(chk<T>(0)) == sizeof(yes);
+};
+
+AST_MATCHER_P(Decl, hasDeclContext, internal::Matcher<Decl>, InnerMatcher) {
+ return InnerMatcher.matches(*Decl::castFromDeclContext(Node.getDeclContext()), Finder, Builder);
+}
+
+AST_POLYMORPHIC_MATCHER_P(hasDecl2, internal::Matcher<Decl>, InnerMatcher) {
+ TOOLING_COMPILE_ASSERT((has_getDecl<NodeType>::value),
+ hasDeclContext_NodeType_has_no_support_for_getDecl);
+ return InnerMatcher.matches(*Node.getDecl(), Finder, Builder);
+}
+
+AST_MATCHER_P(ElaboratedType, hasSpecifier, internal::Matcher<NestedNameSpecifier>, InnerMatcher) {
+ return InnerMatcher.matches(*Node.getQualifier(), Finder, Builder);
+}
+
+AST_MATCHER_P(ElaboratedType, namesType, internal::Matcher<QualType>, InnerMatcher) {
+ return InnerMatcher.matches(Node.getNamedType(), Finder, Builder);
+}
+
+AST_MATCHER(NamedDecl, hasStdIteratorName) {
+ static const char *IteratorNames[] = {
+ "iterator",
+ "reverse_iterator",
+ "const_iterator",
+ "const_reverse_iterator"
+ };
+
+ const std::string FullNameString = Node.getName();
+ for (unsigned int i = 0;
+ i < sizeof(IteratorNames)/sizeof(IteratorNames[0]);
+ ++i) {
+ if (FullNameString == IteratorNames[i]) {
+ return true;
+ }
+ }
+ return false;
+}
+
+AST_MATCHER(NamedDecl, hasStdContainerName) {
+ static const char *ContainerNames[] = {
+ "std::array",
+ "std::deque",
+ "std::forward_list",
+ "std::list",
+ "std::vector",
+
+ "std::map",
+ "std::multimap",
+ "std::set",
+ "std::multiset",
+
+ "std::unordered_map",
+ "std::unordered_multimap",
+ "std::unordered_set",
+ "std::unordered_multiset",
+
+ "std::queue",
+ "std::priority_queue",
+ "std::stack"
+ };
+
+ const std::string FullNameString = Node.getQualifiedNameAsString();
+ for (unsigned int i = 0;
+ i < sizeof(ContainerNames)/sizeof(ContainerNames[0]);
+ ++i) {
+ if (FullNameString == ContainerNames[i]) {
+ return true;
+ }
+ }
+ return false;
+}
+
} // namespace ast_matchers
} // namespace clang
+
DeclarationMatcher makeIteratorMatcher() {
- // FIXME: Instead of looking for traits, should test for existence of
- // std::iterator_traits<T> which will catch pointers used as iterators as
- // well.
return varDecl(allOf(hasWrittenNonListInitializer(),
- hasType(recordDecl(anyOf(
- isDerivedFrom("std::iterator"),
- allOf(has(namedDecl(hasName("value_type"))),
- has(namedDecl(hasName("difference_type"))),
- has(namedDecl(hasName("iterator_category"))),
- has(namedDecl(hasName("reference"))),
- has(namedDecl(hasName("pointer"))))))),
- // Skip type-specifiers that are already 'auto'.
- unless(hasType(autoType())))).bind(DeclNodeId);
+ unless(hasType(autoType())),
+ hasType(
+ hasSugar(
+ anyOf(
+ typedefType(
+ hasDecl2(
+ allOf(
+ namedDecl(hasStdIteratorName()),
+ hasDeclContext(
+ recordDecl(hasStdContainerName())
+ )
+ )
+ )
+ ),
+ recordType(
+ hasDecl2(
+ allOf(
+ namedDecl(hasStdIteratorName()),
+ hasDeclContext(
+ recordDecl(hasStdContainerName())
+ )
+ )
+ )
+ ),
+ elaboratedType(
+ allOf(
+ hasSpecifier(
+ specifiesType(
+ recordType(
+ hasDecl2(
+ namedDecl(hasStdIteratorName())
+ )
+ )
+ )
+ ),
+ namesType(
+ recordType(
+ hasDecl2(
+ namedDecl(hasStdContainerName())
+ )
+ )
+ )
+ )
+ )
+ )
+ )
+ )
+ )).bind(DeclNodeId);
+ //return varDecl(allOf(hasWrittenNonListInitializer(),
+ // hasType(recordDecl(anyOf(
+ // isDerivedFrom("std::iterator"),
+ // allOf(has(namedDecl(hasName("value_type"))),
+ // has(namedDecl(hasName("difference_type"))),
+ // has(namedDecl(hasName("iterator_category"))),
+ // has(namedDecl(hasName("reference"))),
+ // has(namedDecl(hasName("pointer"))))))),
+ // // Skip type-specifiers that are already 'auto'.
+ // unless(hasType(autoType())))).bind(DeclNodeId);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
