[PATCH] Fix UseNullptr fail to replace c-style explicit cast in a return statement.

[Ariel Bernal]

Hi tareqsiraj, revane,

This happens whenever there is a c-style explicit cast to nullptr not 
surrounded by parentheses following a return statement.

- Added a white space before nullptr if the character before is alphanumeric 
when replacing the null pointer expression.
- Simplified visitor.
- Added tests

http://llvm-reviews.chandlerc.com/D639

Files:
  cpp11-migrate/UseNullptr/NullptrActions.cpp
  test/cpp11-migrate/UseNullptr/basic.cpp

Index: cpp11-migrate/UseNullptr/NullptrActions.cpp
===================================================================
--- cpp11-migrate/UseNullptr/NullptrActions.cpp
+++ cpp11-migrate/UseNullptr/NullptrActions.cpp
@@ -20,6 +20,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 
+#include "clang/Basic/CharInfo.h"
 #include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
@@ -42,7 +43,14 @@
                         SourceLocation StartLoc, SourceLocation EndLoc) {
   if (SM.isFromSameFile(StartLoc, EndLoc) && SM.isFromMainFile(StartLoc)) {
     CharSourceRange Range(SourceRange(StartLoc, EndLoc), true);
-    Replace.insert(tooling::Replacement(SM, Range, "nullptr"));
+    // Add a space if nullptr follows an alphanumeric character. This happens
+    // whenever there is an c-style explicit cast to nullptr not surrounded by
+    // parentheses and right beside a return statement.
+    SourceLocation PreviousLocation = StartLoc.getLocWithOffset(-1);
+    if (isAlphanumeric(*FullSourceLoc(PreviousLocation, 
SM).getCharacterData()))
+      Replace.insert(tooling::Replacement(SM, Range, " nullptr"));
+    else
+      Replace.insert(tooling::Replacement(SM, Range, "nullptr"));
     return true;
   } else
     return false;
@@ -93,19 +101,11 @@
   // within a cast expression.
   bool VisitStmt(Stmt *S) {
     CastExpr *C = dyn_cast<CastExpr>(S);
-
     if (!C) {
       ResetFirstSubExpr();
       return true;
     } else if (!FirstSubExpr) {
-      // 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();
+        FirstSubExpr = C->getSubExpr()->IgnoreParens();
     }
 
     if (C->getCastKind() == CK_NullToPointer ||
Index: test/cpp11-migrate/UseNullptr/basic.cpp
===================================================================
--- test/cpp11-migrate/UseNullptr/basic.cpp
+++ test/cpp11-migrate/UseNullptr/basic.cpp
@@ -180,6 +180,23 @@
   // CHECK: return g_null;
 }
 
+int *test_function_return_cast1() {
+  return(int)0;
+  // CHECK: return nullptr;
+}
+
+const int *test_function_return_cast2() {
+  return(int)0;
+  // CHECK: return nullptr;
+}
+
+int *test_function_return_cast3() {
+  #define RET return
+  RET(int)0;
+  // CHECK: RET nullptr;
+  #undef RET
+}
+
 // Test parentheses expressions resulting in a nullptr.
 int *test_parentheses_expression1() {
   return(0);

Index: cpp11-migrate/UseNullptr/NullptrActions.cpp
===================================================================
--- cpp11-migrate/UseNullptr/NullptrActions.cpp
+++ cpp11-migrate/UseNullptr/NullptrActions.cpp
@@ -20,6 +20,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 
+#include "clang/Basic/CharInfo.h"
 #include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
@@ -42,7 +43,14 @@
                         SourceLocation StartLoc, SourceLocation EndLoc) {
   if (SM.isFromSameFile(StartLoc, EndLoc) && SM.isFromMainFile(StartLoc)) {
     CharSourceRange Range(SourceRange(StartLoc, EndLoc), true);
-    Replace.insert(tooling::Replacement(SM, Range, "nullptr"));
+    // Add a space if nullptr follows an alphanumeric character. This happens
+    // whenever there is an c-style explicit cast to nullptr not surrounded by
+    // parentheses and right beside a return statement.
+    SourceLocation PreviousLocation = StartLoc.getLocWithOffset(-1);
+    if (isAlphanumeric(*FullSourceLoc(PreviousLocation, SM).getCharacterData()))
+      Replace.insert(tooling::Replacement(SM, Range, " nullptr"));
+    else
+      Replace.insert(tooling::Replacement(SM, Range, "nullptr"));
     return true;
   } else
     return false;
@@ -93,19 +101,11 @@
   // within a cast expression.
   bool VisitStmt(Stmt *S) {
     CastExpr *C = dyn_cast<CastExpr>(S);
-
     if (!C) {
       ResetFirstSubExpr();
       return true;
     } else if (!FirstSubExpr) {
-      // 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();
+        FirstSubExpr = C->getSubExpr()->IgnoreParens();
     }
 
     if (C->getCastKind() == CK_NullToPointer ||
Index: test/cpp11-migrate/UseNullptr/basic.cpp
===================================================================
--- test/cpp11-migrate/UseNullptr/basic.cpp
+++ test/cpp11-migrate/UseNullptr/basic.cpp
@@ -180,6 +180,23 @@
   // CHECK: return g_null;
 }
 
+int *test_function_return_cast1() {
+  return(int)0;
+  // CHECK: return nullptr;
+}
+
+const int *test_function_return_cast2() {
+  return(int)0;
+  // CHECK: return nullptr;
+}
+
+int *test_function_return_cast3() {
+  #define RET return
+  RET(int)0;
+  // CHECK: RET nullptr;
+  #undef RET
+}
+
 // Test parentheses expressions resulting in a nullptr.
 int *test_parentheses_expression1() {
   return(0);

_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits