Hi tareqsiraj, revane,
Previously UseNullptr matched separately implicit and explicit casts to
nullptr, now it matches casts that either are implicit casts to nullptr or have
an implicit cast to nullptr within.
Also fixes PR15572 since the same macro replacement logic is applied to
implicit and explicit casts.
http://llvm-reviews.chandlerc.com/D604
Files:
cpp11-migrate/UseNullptr/NullptrActions.cpp
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
@@ -28,8 +28,8 @@
namespace {
-/// \brief Replaces the provided range with the text "nullptr", but only if
-/// the start and end location are both in main file.
+/// \brief Replaces the provided range with the text "nullptr", but only if the
+/// start and end location are both in main file.
/// Returns true if and only if a replacement was made.
bool ReplaceWithNullptr(tooling::Replacements &Replace, SourceManager &SM,
SourceLocation StartLoc, SourceLocation EndLoc) {
@@ -43,21 +43,23 @@
}
-/// \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,
- unsigned &AcceptedChanges)
- : Replace(R), SM(SM), AcceptedChanges(AcceptedChanges), FirstSubExpr(0) {}
+ const LangOptions &LangOpts, unsigned &AcceptedChanges)
+ : Replace(R), SM(SM), LangOpts(LangOpts),
+ AcceptedChanges(AcceptedChanges), FirstSubExpr(0) {}
// Only VisitStmt is overridden as we shouldn't find other base AST types
// within a cast expression.
@@ -68,19 +70,46 @@
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 ImmediateMacroName =
+ clang::Lexer::getImmediateMacroName(StartLoc, SM, LangOpts);
+
+ if (ImmediateMacroName != "NULL")
+ return true;
+
+ SourceLocation MacroCallerStartLoc =
+ SM.getImmediateMacroCallerLoc(StartLoc);
+ SourceLocation MacroCallerEndLoc =
+ SM.getImmediateMacroCallerLoc(EndLoc);
+
+ if (MacroCallerStartLoc.isFileID() && MacroCallerEndLoc.isFileID()) {
+ StartLoc = SM.getFileLoc(StartLoc);
+ EndLoc = SM.getFileLoc(EndLoc);
+ } else
+ return true;
+ }
AcceptedChanges +=
ReplaceWithNullptr(Replace, SM, StartLoc, EndLoc) ? 1 : 0;
@@ -97,6 +126,7 @@
private:
tooling::Replacements &Replace;
SourceManager &SM;
+ const LangOptions &LangOpts;
unsigned &AcceptedChanges;
Expr *FirstSubExpr;
};
@@ -106,44 +136,12 @@
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);
+ NullCast->dumpColor();
+ // Given an explicit cast with an implicit null-to-pointer cast within use
+ // CastSequenceVisitor to identify implicit casts and sequences of explicit
+ // casts that can be converted into 'nullptr'.
+ CastSequenceVisitor Visitor(Replace, SM, Result.Context->getLangOpts(),
+ 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 ImmediateMacroName = clang::Lexer::getImmediateMacroName(
- StartLoc, SM, Result.Context->getLangOpts());
- if (ImmediateMacroName != "NULL")
- return;
-
- SourceLocation MacroCallerStartLoc =
- SM.getImmediateMacroCallerLoc(StartLoc);
- SourceLocation MacroCallerEndLoc = SM.getImmediateMacroCallerLoc(EndLoc);
-
- if (MacroCallerStartLoc.isFileID() && MacroCallerEndLoc.isFileID()) {
- StartLoc = SM.getFileLoc(StartLoc);
- EndLoc = SM.getFileLoc(EndLoc);
- } else
- return;
- }
-
- AcceptedChanges +=
- ReplaceWithNullptr(Replace, SM, StartLoc, EndLoc) ? 1 : 0;
- }
}
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
