Incorporated changes made in r178243,
Hi revane, tareqsiraj,
http://llvm-reviews.chandlerc.com/D604
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D604?vs=1462&id=1468#toc
Files:
cpp11-migrate/UseNullptr/NullptrActions.cpp
cpp11-migrate/UseNullptr/NullptrActions.h
cpp11-migrate/UseNullptr/NullptrMatchers.cpp
cpp11-migrate/UseNullptr/NullptrMatchers.h
cpp11-migrate/UseNullptr/UseNullptr.cpp
Index: cpp11-migrate/UseNullptr/NullptrActions.cpp
===================================================================
--- cpp11-migrate/UseNullptr/NullptrActions.cpp
+++ cpp11-migrate/UseNullptr/NullptrActions.cpp
@@ -69,21 +69,25 @@
}
}
-/// \brief Looks for a sequences of 0 or more explicit casts with an implicit
-/// null-to-pointer cast within.
+/// \brief Looks for implicit casts as well as sequences of 0 or more explicit
+/// casts with an implicit null-to-pointer cast within.
///
-/// The matcher this visitor is used with will find a top-most explicit cast
-/// (i.e. it has no explicit casts as an ancestor) where an implicit cast is
-/// nested within. However, there is no guarantee that only explicit casts
-/// exist between the found top-most explicit cast and the possibly more than
-/// one nested implicit cast. This visitor finds all cast sequences with an
-/// implicit cast to null within and creates a replacement leaving the
-/// outermost explicit cast unchanged to avoid introducing ambiguities.
+/// The matcher this visitor is used with will find a single implicit cast or a
+/// top-most explicit cast (i.e. it has no explicit casts as an ancestor) where
+/// an implicit cast is nested within. However, there is no guarantee that only
+/// explicit casts exist between the found top-most explicit cast and the
+/// possibly more than one nested implicit cast. This visitor finds all cast
+/// sequences with an implicit cast to null within and creates a replacement
+/// leaving the outermost explicit cast unchanged to avoid introducing
+/// ambiguities.
class CastSequenceVisitor : public RecursiveASTVisitor<CastSequenceVisitor> {
public:
CastSequenceVisitor(tooling::Replacements &R, SourceManager &SM,
+ const LangOptions &LangOpts,
+ const UserMacroNames &UserNullMacros,
unsigned &AcceptedChanges)
- : Replace(R), SM(SM), AcceptedChanges(AcceptedChanges), FirstSubExpr(0) {}
+ : Replace(R), SM(SM), LangOpts(LangOpts), UserNullMacros(UserNullMacros),
+ AcceptedChanges(AcceptedChanges), FirstSubExpr(0) {}
// Only VisitStmt is overridden as we shouldn't find other base AST types
// within a cast expression.
@@ -94,19 +98,44 @@
ResetFirstSubExpr();
return true;
} else if (!FirstSubExpr) {
- // Get the subexpression of the outermost explicit cast
- FirstSubExpr = C->getSubExpr();
+ // Keep parentheses for implicit casts to avoid cases where an implicit
+ // cast within a parentheses expression is right next to a return
+ // statement otherwise get the subexpression of the outermost explicit
+ // cast.
+ if (C->getStmtClass() == Stmt::ImplicitCastExprClass)
+ FirstSubExpr = C->IgnoreParenImpCasts();
+ else
+ FirstSubExpr = C->getSubExpr();
}
if (C->getCastKind() == CK_NullToPointer ||
C->getCastKind() == CK_NullToMemberPointer) {
SourceLocation StartLoc = FirstSubExpr->getLocStart();
SourceLocation EndLoc = FirstSubExpr->getLocEnd();
- // If the start/end location is a macro, get the expansion location.
- StartLoc = SM.getFileLoc(StartLoc);
- EndLoc = SM.getFileLoc(EndLoc);
+ // If the start/end location is a macro argument expansion, get the
+ // expansion location. If its a macro body expansion, check to see if its
+ // coming from a macro called NULL.
+ if (SM.isMacroArgExpansion(StartLoc) && SM.isMacroArgExpansion(EndLoc)) {
+ StartLoc = SM.getFileLoc(StartLoc);
+ EndLoc = SM.getFileLoc(EndLoc);
+ } else if (SM.isMacroBodyExpansion(StartLoc) &&
+ SM.isMacroBodyExpansion(EndLoc)) {
+ llvm::StringRef OutermostMacroName =
+ GetOutermostMacroName(StartLoc, SM, LangOpts);
+
+ // Check to see if the user wants to replace the macro being expanded.
+ bool ReplaceNullMacro =
+ std::find(UserNullMacros.begin(), UserNullMacros.end(),
+ OutermostMacroName) != UserNullMacros.end();
+
+ if (!ReplaceNullMacro)
+ return false;
+
+ StartLoc = SM.getFileLoc(StartLoc);
+ EndLoc = SM.getFileLoc(EndLoc);
+ }
AcceptedChanges +=
ReplaceWithNullptr(Replace, SM, StartLoc, EndLoc) ? 1 : 0;
@@ -123,6 +152,8 @@
private:
tooling::Replacements &Replace;
SourceManager &SM;
+ const LangOptions &LangOpts;
+ const UserMacroNames &UserNullMacros;
unsigned &AcceptedChanges;
Expr *FirstSubExpr;
};
@@ -141,44 +172,11 @@
SourceManager &SM = *Result.SourceManager;
const CastExpr *NullCast = Result.Nodes.getNodeAs<CastExpr>(CastSequence);
- if (NullCast) {
- // Given an explicit cast with an implicit null-to-pointer cast within
- // use CastSequenceVisitor to identify sequences of explicit casts that can
- // be converted into 'nullptr'.
- CastSequenceVisitor Visitor(Replace, SM, AcceptedChanges);
- Visitor.TraverseStmt(const_cast<CastExpr *>(NullCast));
- }
-
- const CastExpr *Cast = Result.Nodes.getNodeAs<CastExpr>(ImplicitCastNode);
- if (Cast) {
- const Expr *E = Cast->IgnoreParenImpCasts();
- SourceLocation StartLoc = E->getLocStart();
- SourceLocation EndLoc = E->getLocEnd();
-
- // If the start/end location is a macro argument expansion, get the
- // expansion location. If its a macro body expansion, check to see if its
- // coming from a macro called NULL.
- if (SM.isMacroArgExpansion(StartLoc) && SM.isMacroArgExpansion(EndLoc)) {
- StartLoc = SM.getFileLoc(StartLoc);
- EndLoc = SM.getFileLoc(EndLoc);
- } else if (SM.isMacroBodyExpansion(StartLoc) &&
- SM.isMacroBodyExpansion(EndLoc)) {
- llvm::StringRef OutermostMacroName =
- GetOutermostMacroName(StartLoc, SM, Result.Context->getLangOpts());
-
- // Check to see if the user wants to replace the macro being expanded.
- bool ReplaceNullMacro =
- std::find(UserNullMacros.begin(), UserNullMacros.end(),
- OutermostMacroName) != UserNullMacros.end();
-
- if (!ReplaceNullMacro)
- return;
-
- StartLoc = SM.getFileLoc(StartLoc);
- EndLoc = SM.getFileLoc(EndLoc);
- }
-
- AcceptedChanges +=
- ReplaceWithNullptr(Replace, SM, StartLoc, EndLoc) ? 1 : 0;
- }
+ assert(NullCast && "Bad Calback. No node provided");
+ // Given an implicit null-ptr cast or an explicit cast with an implicit
+ // null-to-pointer cast within use CastSequenceVisitor to identify sequences
+ // of explicit casts that can be converted into 'nullptr'.
+ CastSequenceVisitor Visitor(Replace, SM, Result.Context->getLangOpts(),
+ UserNullMacros, AcceptedChanges);
+ Visitor.TraverseStmt(const_cast<CastExpr *>(NullCast));
}
Index: cpp11-migrate/UseNullptr/NullptrActions.h
===================================================================
--- cpp11-migrate/UseNullptr/NullptrActions.h
+++ cpp11-migrate/UseNullptr/NullptrActions.h
@@ -19,6 +19,9 @@
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Tooling/Refactoring.h"
+// The type for user-defined macro names that behave like NULL
+typedef llvm::SmallVector<llvm::StringRef, 1> UserMacroNames;
+
/// \brief The callback to be used for nullptr migration matchers.
///
class NullptrFixer : public clang::ast_matchers::MatchFinder::MatchCallback {
@@ -33,7 +36,7 @@
private:
clang::tooling::Replacements &Replace;
unsigned &AcceptedChanges;
- llvm::SmallVector<llvm::StringRef, 1> UserNullMacros;
+ UserMacroNames UserNullMacros;
};
#endif // LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_NULLPTR_ACTIONS_H
Index: cpp11-migrate/UseNullptr/NullptrMatchers.cpp
===================================================================
--- cpp11-migrate/UseNullptr/NullptrMatchers.cpp
+++ cpp11-migrate/UseNullptr/NullptrMatchers.cpp
@@ -18,7 +18,6 @@
using namespace clang::ast_matchers;
using namespace clang;
-const char *ImplicitCastNode = "cast";
const char *CastSequence = "sequence";
namespace clang {
@@ -47,30 +46,22 @@
} // end namespace ast_matchers
} // end namespace clang
-StatementMatcher makeImplicitCastMatcher() {
- return implicitCastExpr(
- isNullToPointer(),
- unless(hasAncestor(explicitCastExpr())),
- unless(
- hasSourceExpression(
- hasType(sugaredNullptrType())
- )
- )
- ).bind(ImplicitCastNode);
-}
-
StatementMatcher makeCastSequenceMatcher() {
- return explicitCastExpr(
+ StatementMatcher ImplicitCastToNull =
+ implicitCastExpr(
+ isNullToPointer(),
+ unless(
+ hasSourceExpression(
+ hasType(sugaredNullptrType())
+ )
+ )
+ );
+
+ return castExpr(
unless(hasAncestor(explicitCastExpr())),
- hasDescendant(
- implicitCastExpr(
- isNullToPointer(),
- unless(
- hasSourceExpression(
- hasType(sugaredNullptrType())
- )
- )
- )
+ anyOf(
+ hasDescendant(ImplicitCastToNull),
+ ImplicitCastToNull
)
).bind(CastSequence);
}
Index: cpp11-migrate/UseNullptr/NullptrMatchers.h
===================================================================
--- cpp11-migrate/UseNullptr/NullptrMatchers.h
+++ cpp11-migrate/UseNullptr/NullptrMatchers.h
@@ -18,21 +18,13 @@
#include "clang/ASTMatchers/ASTMatchers.h"
// Names to bind with matched expressions.
-extern const char *ImplicitCastNode;
extern const char *CastSequence;
-/// \brief Create a matcher to find the implicit casts clang inserts
-/// when writing null to a pointer.
-///
-/// However, don't match those implicit casts that have explicit casts as
-/// an ancestor. Explicit casts are handled by makeCastSequenceMatcher().
-clang::ast_matchers::StatementMatcher makeImplicitCastMatcher();
-
-/// \brief Create a matcher that finds the head of a sequence of nested explicit
-/// casts that have an implicit cast to null within.
-///
-/// This matcher is necessary so that an entire sequence of explicit casts can
-/// be replaced instead of just the inner-most implicit cast.
+/// \brief Create a matcher that finds implicit casts as well as the head of a
+/// sequence of zero or more nested explicit casts that have an implicit cast
+/// to null within.
+/// Finding sequences of explict casts is necessary so that an entire sequence
+/// can be replaced instead of just the inner-most implicit cast.
clang::ast_matchers::StatementMatcher makeCastSequenceMatcher();
#endif // LLVM_TOOLS_CLANG_TOOLS_EXTRA_CPP11_MIGRATE_USE_NULLPTR_MATCHERS_H
Index: cpp11-migrate/UseNullptr/UseNullptr.cpp
===================================================================
--- cpp11-migrate/UseNullptr/UseNullptr.cpp
+++ cpp11-migrate/UseNullptr/UseNullptr.cpp
@@ -45,7 +45,6 @@
AcceptedChanges,
MaxRisk);
- Finder.addMatcher(makeImplicitCastMatcher(), &Fixer);
Finder.addMatcher(makeCastSequenceMatcher(), &Fixer);
if (int result = UseNullptrTool.run(newFrontendActionFactory(&Finder))) {
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
