[clang-tools-extra] r182736 - Fix UseAuto replacing declaration lists with new expressions
Ariel J. Bernal
ariel.j.bernal at intel.com
Mon May 27 07:30:24 PDT 2013
Author: ajbernal
Date: Mon May 27 09:30:23 2013
New Revision: 182736
URL: http://llvm.org/viewvc/llvm-project?rev=182736&view=rev
Log:
Fix UseAuto replacing declaration lists with new expressions
UseAuto used to replace declarion lists with new expressons where some
variable were not initialized with new.
This fix checks that every DeclStmt has a VarDecl with an initializer and it
also ensures that all declarations have the same type.
Added tests for multiple declarations and for typedefs.
Modified:
clang-tools-extra/trunk/cpp11-migrate/UseAuto/UseAutoActions.cpp
clang-tools-extra/trunk/cpp11-migrate/UseAuto/UseAutoMatchers.cpp
clang-tools-extra/trunk/cpp11-migrate/UseAuto/UseAutoMatchers.h
clang-tools-extra/trunk/test/cpp11-migrate/UseAuto/new.cpp
Modified: clang-tools-extra/trunk/cpp11-migrate/UseAuto/UseAutoActions.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/UseAuto/UseAutoActions.cpp?rev=182736&r1=182735&r2=182736&view=diff
==============================================================================
--- clang-tools-extra/trunk/cpp11-migrate/UseAuto/UseAutoActions.cpp (original)
+++ clang-tools-extra/trunk/cpp11-migrate/UseAuto/UseAutoActions.cpp Mon May 27 09:30:23 2013
@@ -20,7 +20,6 @@ using namespace clang::ast_matchers;
using namespace clang::tooling;
using namespace clang;
-
void IteratorReplacer::run(const MatchFinder::MatchResult &Result) {
const DeclStmt *D = Result.Nodes.getNodeAs<DeclStmt>(IteratorDeclStmtId);
assert(D && "Bad Callback. No node provided");
@@ -49,16 +48,16 @@ void IteratorReplacer::run(const MatchFi
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.
+ // conversion from a different type. Could also mean an explicit
+ // conversion from the same type but that's pretty rare.
return;
if (const CXXConstructExpr *NestedConstruct = dyn_cast<CXXConstructExpr>(E))
// If we ran into an implicit conversion constructor, can't convert.
//
// FIXME: The following only checks if the constructor can be used
- // implicitly, not if it actually was. Cases where the converting constructor
- // was used explicitly won't get converted.
+ // implicitly, not if it actually was. Cases where the converting
+ // constructor was used explicitly won't get converted.
if (NestedConstruct->getConstructor()->isConvertingConstructor(false))
return;
if (!Result.Context->hasSameType(V->getType(), E->getType()))
@@ -78,46 +77,69 @@ void IteratorReplacer::run(const MatchFi
}
void NewReplacer::run(const MatchFinder::MatchResult &Result) {
- const VarDecl *D = Result.Nodes.getNodeAs<VarDecl>(DeclWithNewId);
+ const DeclStmt *D = Result.Nodes.getNodeAs<DeclStmt>(DeclWithNewId);
assert(D && "Bad Callback. No node provided");
SourceManager &SM = *Result.SourceManager;
if (!SM.isFromMainFile(D->getLocStart()))
return;
- const CXXNewExpr *NewExpr = Result.Nodes.getNodeAs<CXXNewExpr>(NewExprId);
- assert(NewExpr && "Bad Callback. No CXXNewExpr bound");
+ const VarDecl *FirstDecl = cast<VarDecl>(*D->decl_begin());
+ // Ensure that there is at least one VarDecl within de DeclStmt.
+ assert(FirstDecl && "No VarDecl provided");
- // If declaration and initializer have exactly the same type, just replace
- // with 'auto'.
- if (Result.Context->hasSameType(D->getType(), NewExpr->getType())) {
- TypeLoc TL = D->getTypeSourceInfo()->getTypeLoc();
- CharSourceRange Range(TL.getSourceRange(), /*IsTokenRange=*/ true);
- // Space after 'auto' to handle cases where the '*' in the pointer type
- // is next to the identifier. This avoids changing 'int *p' into 'autop'.
- Replace.insert(tooling::Replacement(SM, Range, "auto "));
- ++AcceptedChanges;
- return;
- }
+ const QualType FirstDeclType = FirstDecl->getType().getCanonicalType();
- // If the CV qualifiers for the pointer differ then we still use auto, just
- // need to leave the qualifier behind.
- if (Result.Context->hasSameUnqualifiedType(D->getType(),
- NewExpr->getType())) {
- TypeLoc TL = D->getTypeSourceInfo()->getTypeLoc();
- CharSourceRange Range(TL.getSourceRange(), /*IsTokenRange=*/ true);
- // Space after 'auto' to handle cases where the '*' in the pointer type
- // is next to the identifier. This avoids changing 'int *p' into 'autop'.
- Replace.insert(tooling::Replacement(SM, Range, "auto "));
- ++AcceptedChanges;
- return;
- }
+ std::vector<SourceLocation> StarLocations;
+ for (clang::DeclStmt::const_decl_iterator DI = D->decl_begin(),
+ DE = D->decl_end();
+ DI != DE; ++DI) {
+
+ const VarDecl *V = cast<VarDecl>(*DI);
+ // Ensure that every DeclStmt child is a VarDecl.
+ assert(V && "No VarDecl provided");
+
+ const CXXNewExpr *NewExpr =
+ cast<CXXNewExpr>(V->getInit()->IgnoreParenImpCasts());
+ // Ensure that every VarDecl has a CXXNewExpr initializer.
+ assert(NewExpr && "No CXXNewExpr provided");
+
+ // If VarDecl and Initializer have mismatching unqualified types.
+ if (!Result.Context->hasSameUnqualifiedType(V->getType(),
+ NewExpr->getType()))
+ return;
- // The VarDecl and Initializer have mismatching types.
- return;
+ // Remove explicitly written '*' from declarations where there's more than
+ // one declaration in the declaration list.
+ if (DI == D->decl_begin())
+ continue;
+
+ // All subsequent delcarations should match the same non-decorated type.
+ if (FirstDeclType != V->getType().getCanonicalType())
+ return;
+ PointerTypeLoc Q =
+ V->getTypeSourceInfo()->getTypeLoc().getAs<PointerTypeLoc>();
+ while (!Q.isNull()) {
+ StarLocations.push_back(Q.getStarLoc());
+ Q = Q.getNextTypeLoc().getAs<PointerTypeLoc>();
+ }
+ }
+
+ // Remove '*' from declarations using the saved star locations.
+ for (std::vector<SourceLocation>::iterator I = StarLocations.begin(),
+ E = StarLocations.end();
+ I != E; ++I) {
+ Replace.insert(tooling::Replacement(SM, *I, 1, ""));
+ }
// FIXME: There is, however, one case we can address: when the VarDecl
// pointee is the same as the initializer, just more CV-qualified. However,
// TypeLoc information is not reliable where CV qualifiers are concerned so
// we can't do anything about this case for now.
+ CharSourceRange Range(
+ FirstDecl->getTypeSourceInfo()->getTypeLoc().getSourceRange(), true);
+ // Space after 'auto' to handle cases where the '*' in the pointer type
+ // is next to the identifier. This avoids changing 'int *p' into 'autop'.
+ Replace.insert(tooling::Replacement(SM, Range, "auto "));
+ ++AcceptedChanges;
}
Modified: clang-tools-extra/trunk/cpp11-migrate/UseAuto/UseAutoMatchers.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/UseAuto/UseAutoMatchers.cpp?rev=182736&r1=182735&r2=182736&view=diff
==============================================================================
--- clang-tools-extra/trunk/cpp11-migrate/UseAuto/UseAutoMatchers.cpp (original)
+++ clang-tools-extra/trunk/cpp11-migrate/UseAuto/UseAutoMatchers.cpp Mon May 27 09:30:23 2013
@@ -258,29 +258,29 @@ StatementMatcher makeIteratorDeclMatcher
).bind(IteratorDeclStmtId);
}
-DeclarationMatcher makeDeclWithNewMatcher() {
- return varDecl(
- hasInitializer(
- ignoringParenImpCasts(
- newExpr().bind(NewExprId)
- )
- ),
+StatementMatcher makeDeclWithNewMatcher() {
+ return declStmt(
+ has(varDecl()),
+ unless(has(varDecl(
+ anyOf(
+ unless(hasInitializer(
+ ignoringParenImpCasts(newExpr())
+ )),
+ // FIXME: TypeLoc information is not reliable where CV qualifiers are
+ // concerned so these types can't be handled for now.
+ hasType(pointerType(pointee(hasCanonicalType(hasLocalQualifiers())))),
- // FIXME: TypeLoc information is not reliable where CV qualifiers are
- // concerned so these types can't be handled for now.
- unless(hasType(pointerType(pointee(hasLocalQualifiers())))),
-
- // FIXME: Handle function pointers. For now we ignore them because
- // the replacement replaces the entire type specifier source range
- // which includes the identifier.
- unless(
- hasType(
- pointsTo(
- pointsTo(
- parenType(innerType(functionType()))
- )
- )
- )
- )
- ).bind(DeclWithNewId);
+ // FIXME: Handle function pointers. For now we ignore them because
+ // the replacement replaces the entire type specifier source range
+ // which includes the identifier.
+ hasType(
+ pointsTo(
+ pointsTo(
+ parenType(innerType(functionType()))
+ )
+ )
+ )
+ )
+ )))
+ ).bind(DeclWithNewId);
}
Modified: clang-tools-extra/trunk/cpp11-migrate/UseAuto/UseAutoMatchers.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/UseAuto/UseAutoMatchers.h?rev=182736&r1=182735&r2=182736&view=diff
==============================================================================
--- clang-tools-extra/trunk/cpp11-migrate/UseAuto/UseAutoMatchers.h (original)
+++ clang-tools-extra/trunk/cpp11-migrate/UseAuto/UseAutoMatchers.h Mon May 27 09:30:23 2013
@@ -28,6 +28,6 @@ clang::ast_matchers::StatementMatcher ma
/// \brief Create a matcher that matches variable declarations that are
/// initialized by a C++ new expression.
-clang::ast_matchers::DeclarationMatcher makeDeclWithNewMatcher();
+clang::ast_matchers::StatementMatcher makeDeclWithNewMatcher();
#endif // LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_USE_AUTO_MATCHERS_H
Modified: clang-tools-extra/trunk/test/cpp11-migrate/UseAuto/new.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/cpp11-migrate/UseAuto/new.cpp?rev=182736&r1=182735&r2=182736&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/cpp11-migrate/UseAuto/new.cpp (original)
+++ clang-tools-extra/trunk/test/cpp11-migrate/UseAuto/new.cpp Mon May 27 09:30:23 2013
@@ -49,4 +49,49 @@ int main(int argc, char **argv) {
MyType *g{new MyType};
// CHECK: MyType *g{new MyType};
+
+ // Test for declaration lists.
+ {
+ MyType *a = new MyType(), *b = new MyType(), *c = new MyType();
+ // CHECK: auto a = new MyType(), b = new MyType(), c = new MyType();
+
+ // Non-initialized declaration should not be transformed.
+ MyType *d = new MyType(), *e;
+ // CHECK: MyType *d = new MyType(), *e;
+
+ MyType **f = new MyType*(), **g = new MyType*();
+ // CHECK: auto f = new MyType*(), g = new MyType*();
+
+ // Mismatching types in declaration lists should not be transformed.
+ MyType *h = new MyType(), **i = new MyType*();
+ // CHECK: MyType *h = new MyType(), **i = new MyType*();
+
+ // '*' shouldn't be removed in case of mismatching types with multiple
+ // declarations.
+ MyType *j = new MyType(), *k = new MyType(), **l = new MyType*();
+ // CHECK: MyType *j = new MyType(), *k = new MyType(), **l = new MyType*();
+ }
+
+ // Test for typedefs.
+ {
+ typedef int * int_p;
+
+ int_p a = new int;
+ // CHECK: auto a = new int;
+ int_p *b = new int*;
+ // CHECK: auto b = new int*;
+
+ // Test for typedefs in declarations lists.
+ int_p c = new int, d = new int;
+ // CHECK: auto c = new int, d = new int;
+
+ // Different types should not be transformed.
+ int_p e = new int, *f = new int_p;
+ // CHECK: int_p e = new int, *f = new int_p;
+
+ int_p *g = new int*, *h = new int_p;
+ // CHECK: auto g = new int*, h = new int_p;
+ }
+
+ return 0;
}
More information about the cfe-commits
mailing list