[PATCH] PR15279: -use-nullptr behaves badly with assert()

[Tareq A. Siraj]

Hi klimek,

If a cast expression (NullToPointer) is detected in a function-like
macro parameter, we should use the spelling location instead of the
expansion location. Using SourceManager::getFileLoc() fixes this
problem.

Also added testcases for this bug.

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

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
@@ -57,6 +57,11 @@
       SourceLocation StartLoc = FirstCast->getLocStart();
       SourceLocation EndLoc = FirstCast->getLocEnd();
 
+      if (StartLoc.isMacroID())
+        StartLoc = SM.getFileLoc(StartLoc);
+      if (EndLoc.isMacroID())
+        EndLoc = SM.getFileLoc(EndLoc);
+
       if (SM.getFileID(StartLoc) == SM.getFileID(EndLoc) &&
           SM.isFromMainFile(StartLoc) && SM.isFromMainFile(EndLoc)) {
         CharSourceRange Range(SourceRange(StartLoc, EndLoc), true);
@@ -101,9 +106,9 @@
       return;
 
     if (StartLoc.isMacroID())
-      StartLoc = SM.getExpansionLoc(StartLoc);
+      StartLoc = SM.getFileLoc(StartLoc);
     if (EndLoc.isMacroID())
-      EndLoc = SM.getExpansionLoc(EndLoc);
+      EndLoc = SM.getFileLoc(EndLoc);
 
     if (!SM.isFromMainFile(StartLoc) || !SM.isFromMainFile(EndLoc))
       return;
Index: test/cpp11-migrate/UseNullptr/basic.cpp
===================================================================
--- test/cpp11-migrate/UseNullptr/basic.cpp
+++ test/cpp11-migrate/UseNullptr/basic.cpp
@@ -178,3 +178,29 @@
   return g_null;
   // CHECK: return g_null;
 }
+
+// Test function-like macros where the parameter to the macro (expression)
+// results in a nullptr.
+void __dummy_assert_fail() {}
+
+void test_function_like_macro1() {
+  // This tests that the CastSequenceVisitor is working properly.
+#define my_assert(expr) \
+  ((expr) ? static_cast<void>(expr) : __dummy_assert_fail())
+
+  int *p;
+  my_assert(p != 0);
+  //CHECK: my_assert(p != nullptr);
+#undef my_assert
+}
+
+void test_function_like_macro2() {
+  // This tests that the implicit cast is working properly.
+#define my_macro(expr) \
+  (expr)
+
+  int *p;
+  my_macro(p != 0);
+  //CHECK: my_macro(p != nullptr);
+#undef my_macro
+}

Index: cpp11-migrate/UseNullptr/NullptrActions.cpp
===================================================================
--- cpp11-migrate/UseNullptr/NullptrActions.cpp
+++ cpp11-migrate/UseNullptr/NullptrActions.cpp
@@ -57,6 +57,11 @@
       SourceLocation StartLoc = FirstCast->getLocStart();
       SourceLocation EndLoc = FirstCast->getLocEnd();
 
+      if (StartLoc.isMacroID())
+        StartLoc = SM.getFileLoc(StartLoc);
+      if (EndLoc.isMacroID())
+        EndLoc = SM.getFileLoc(EndLoc);
+
       if (SM.getFileID(StartLoc) == SM.getFileID(EndLoc) &&
           SM.isFromMainFile(StartLoc) && SM.isFromMainFile(EndLoc)) {
         CharSourceRange Range(SourceRange(StartLoc, EndLoc), true);
@@ -101,9 +106,9 @@
       return;
 
     if (StartLoc.isMacroID())
-      StartLoc = SM.getExpansionLoc(StartLoc);
+      StartLoc = SM.getFileLoc(StartLoc);
     if (EndLoc.isMacroID())
-      EndLoc = SM.getExpansionLoc(EndLoc);
+      EndLoc = SM.getFileLoc(EndLoc);
 
     if (!SM.isFromMainFile(StartLoc) || !SM.isFromMainFile(EndLoc))
       return;
Index: test/cpp11-migrate/UseNullptr/basic.cpp
===================================================================
--- test/cpp11-migrate/UseNullptr/basic.cpp
+++ test/cpp11-migrate/UseNullptr/basic.cpp
@@ -178,3 +178,29 @@
   return g_null;
   // CHECK: return g_null;
 }
+
+// Test function-like macros where the parameter to the macro (expression)
+// results in a nullptr.
+void __dummy_assert_fail() {}
+
+void test_function_like_macro1() {
+  // This tests that the CastSequenceVisitor is working properly.
+#define my_assert(expr) \
+  ((expr) ? static_cast<void>(expr) : __dummy_assert_fail())
+
+  int *p;
+  my_assert(p != 0);
+  //CHECK: my_assert(p != nullptr);
+#undef my_assert
+}
+
+void test_function_like_macro2() {
+  // This tests that the implicit cast is working properly.
+#define my_macro(expr) \
+  (expr)
+
+  int *p;
+  my_macro(p != 0);
+  //CHECK: my_macro(p != nullptr);
+#undef my_macro
+}

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