[clang-tools-extra] r178494 - Refactor Usenullptr matcher to avoid duplication
Ariel J. Bernal
ariel.j.bernal at intel.com
Mon Apr 1 13:09:29 PDT 2013
Author: ajbernal
Date: Mon Apr 1 15:09:29 2013
New Revision: 178494
URL: http://llvm.org/viewvc/llvm-project?rev=178494&view=rev
Log:
Refactor Usenullptr matcher to avoid duplication
Previously UseNullptr matched separately implicit and explicit casts to nullptr,
now it matches casts that either are implict 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.
Modified:
clang-tools-extra/trunk/cpp11-migrate/UseNullptr/NullptrActions.cpp
clang-tools-extra/trunk/cpp11-migrate/UseNullptr/NullptrActions.h
clang-tools-extra/trunk/cpp11-migrate/UseNullptr/NullptrMatchers.cpp
clang-tools-extra/trunk/cpp11-migrate/UseNullptr/NullptrMatchers.h
clang-tools-extra/trunk/cpp11-migrate/UseNullptr/UseNullptr.cpp
clang-tools-extra/trunk/test/cpp11-migrate/UseNullptr/macros.cpp
Modified: clang-tools-extra/trunk/cpp11-migrate/UseNullptr/NullptrActions.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/UseNullptr/NullptrActions.cpp?rev=178494&r1=178493&r2=178494&view=diff
==============================================================================
--- clang-tools-extra/trunk/cpp11-migrate/UseNullptr/NullptrActions.cpp (original)
+++ clang-tools-extra/trunk/cpp11-migrate/UseNullptr/NullptrActions.cpp Mon Apr 1 15:09:29 2013
@@ -69,21 +69,25 @@ llvm::StringRef GetOutermostMacroName(
}
}
-/// \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,8 +98,14 @@ public:
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 ||
@@ -104,9 +114,28 @@ public:
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:
private:
tooling::Replacements &Replace;
SourceManager &SM;
+ const LangOptions &LangOpts;
+ const UserMacroNames &UserNullMacros;
unsigned &AcceptedChanges;
Expr *FirstSubExpr;
};
@@ -141,44 +172,11 @@ void NullptrFixer::run(const ast_matcher
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 Callback. 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));
}
Modified: clang-tools-extra/trunk/cpp11-migrate/UseNullptr/NullptrActions.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/UseNullptr/NullptrActions.h?rev=178494&r1=178493&r2=178494&view=diff
==============================================================================
--- clang-tools-extra/trunk/cpp11-migrate/UseNullptr/NullptrActions.h (original)
+++ clang-tools-extra/trunk/cpp11-migrate/UseNullptr/NullptrActions.h Mon Apr 1 15:09:29 2013
@@ -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 @@ public:
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
Modified: clang-tools-extra/trunk/cpp11-migrate/UseNullptr/NullptrMatchers.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/UseNullptr/NullptrMatchers.cpp?rev=178494&r1=178493&r2=178494&view=diff
==============================================================================
--- clang-tools-extra/trunk/cpp11-migrate/UseNullptr/NullptrMatchers.cpp (original)
+++ clang-tools-extra/trunk/cpp11-migrate/UseNullptr/NullptrMatchers.cpp Mon Apr 1 15:09:29 2013
@@ -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 @@ AST_MATCHER(Type, sugaredNullptrType) {
} // 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);
}
Modified: clang-tools-extra/trunk/cpp11-migrate/UseNullptr/NullptrMatchers.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/UseNullptr/NullptrMatchers.h?rev=178494&r1=178493&r2=178494&view=diff
==============================================================================
--- clang-tools-extra/trunk/cpp11-migrate/UseNullptr/NullptrMatchers.h (original)
+++ clang-tools-extra/trunk/cpp11-migrate/UseNullptr/NullptrMatchers.h Mon Apr 1 15:09:29 2013
@@ -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
Modified: clang-tools-extra/trunk/cpp11-migrate/UseNullptr/UseNullptr.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/UseNullptr/UseNullptr.cpp?rev=178494&r1=178493&r2=178494&view=diff
==============================================================================
--- clang-tools-extra/trunk/cpp11-migrate/UseNullptr/UseNullptr.cpp (original)
+++ clang-tools-extra/trunk/cpp11-migrate/UseNullptr/UseNullptr.cpp Mon Apr 1 15:09:29 2013
@@ -45,7 +45,6 @@ int UseNullptrTransform::apply(const Fil
AcceptedChanges,
MaxRisk);
- Finder.addMatcher(makeImplicitCastMatcher(), &Fixer);
Finder.addMatcher(makeCastSequenceMatcher(), &Fixer);
if (int result = UseNullptrTool.run(newFrontendActionFactory(&Finder))) {
Modified: clang-tools-extra/trunk/test/cpp11-migrate/UseNullptr/macros.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/cpp11-migrate/UseNullptr/macros.cpp?rev=178494&r1=178493&r2=178494&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/cpp11-migrate/UseNullptr/macros.cpp (original)
+++ clang-tools-extra/trunk/test/cpp11-migrate/UseNullptr/macros.cpp Mon Apr 1 15:09:29 2013
@@ -39,8 +39,22 @@ void test_macro_expansion1() {
#undef MACRO_EXPANSION_HAS_NULL
}
+// Test macro expansion with cast sequence, PR15572
void test_macro_expansion2() {
#define MACRO_EXPANSION_HAS_NULL \
+ dummy((int*)0); \
+ side_effect();
+ // CHECK: dummy((int*)0); \
+ // CHECK-NEXT: side_effect();
+
+ MACRO_EXPANSION_HAS_NULL;
+ // CHECK: MACRO_EXPANSION_HAS_NULL;
+
+#undef MACRO_EXPANSION_HAS_NULL
+}
+
+void test_macro_expansion3() {
+#define MACRO_EXPANSION_HAS_NULL \
dummy(NULL); \
side_effect();
// CHECK: dummy(NULL); \
@@ -57,7 +71,7 @@ void test_macro_expansion2() {
#undef MACRO_EXPANSION_HAS_NULL
}
-void test_macro_expansion3() {
+void test_macro_expansion4() {
#define MY_NULL NULL
int *p = MY_NULL;
// CHECK: int *p = MY_NULL;
@@ -91,5 +105,3 @@ void test_function_like_macro2() {
// CHECK: my_macro(p != nullptr);
#undef my_macro
}
-
-
More information about the cfe-commits
mailing list