[PATCH] D14229: Remove unreachable that was reached in modernize-use-nullptr.

[Angel Garcia via cfe-commits]

angelgarcia created this revision.
angelgarcia added a reviewer: alexfh.
angelgarcia added subscribers: cfe-commits, klimek.

When traversing the parent map, the check assumed that all the nodes would be 
either Stmt or Decl. After r251101, this is no longer true: there can be 
TypeLoc and NestedNameSpecifierLoc nodes.

http://reviews.llvm.org/D14229

Files:
  clang-tidy/modernize/UseNullptrCheck.cpp
  test/clang-tidy/modernize-use-nullptr.cpp

Index: test/clang-tidy/modernize-use-nullptr.cpp
===================================================================
--- test/clang-tidy/modernize-use-nullptr.cpp
+++ test/clang-tidy/modernize-use-nullptr.cpp
@@ -184,3 +184,15 @@
   // CHECK-FIXES: a[2] = {ENTRY(nullptr), {nullptr}};
 #undef ENTRY
 }
+
+// One of the ancestor of the cast is a NestedNameSpecifierLoc.
+class NoDef;
+char function(NoDef *p);
+#define F(x) \
+    (sizeof(function(x)) == 1)
+template<class T, T t>
+class C {};
+C<bool, F(0)> c;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use nullptr
+// CHECK-FIXES: C<bool, F(nullptr)> c;
+#undef F
Index: clang-tidy/modernize/UseNullptrCheck.cpp
===================================================================
--- clang-tidy/modernize/UseNullptrCheck.cpp
+++ clang-tidy/modernize/UseNullptrCheck.cpp
@@ -435,12 +435,14 @@
         Loc = D->getLocStart();
       else if (const auto *S = Parent.get<Stmt>())
         Loc = S->getLocStart();
-      else
-        llvm_unreachable("Expected to find Decl or Stmt containing ancestor");
 
-      if (!expandsFrom(Loc, MacroLoc)) {
-        Result = Parent;
-        return true;
+      // TypeLoc and NestedNameSpecifierLoc are members of the parent map. Skip
+      // them and keep going up.
+      if (Loc.isValid()) {
+        if (!expandsFrom(Loc, MacroLoc)) {
+          Result = Parent;
+          return true;
+        }
       }
       Start = Parent;
     }



Index: test/clang-tidy/modernize-use-nullptr.cpp
===================================================================
--- test/clang-tidy/modernize-use-nullptr.cpp
+++ test/clang-tidy/modernize-use-nullptr.cpp
@@ -184,3 +184,15 @@
   // CHECK-FIXES: a[2] = {ENTRY(nullptr), {nullptr}};
 #undef ENTRY
 }
+
+// One of the ancestor of the cast is a NestedNameSpecifierLoc.
+class NoDef;
+char function(NoDef *p);
+#define F(x) \
+    (sizeof(function(x)) == 1)
+template<class T, T t>
+class C {};
+C<bool, F(0)> c;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use nullptr
+// CHECK-FIXES: C<bool, F(nullptr)> c;
+#undef F
Index: clang-tidy/modernize/UseNullptrCheck.cpp
===================================================================
--- clang-tidy/modernize/UseNullptrCheck.cpp
+++ clang-tidy/modernize/UseNullptrCheck.cpp
@@ -435,12 +435,14 @@
         Loc = D->getLocStart();
       else if (const auto *S = Parent.get<Stmt>())
         Loc = S->getLocStart();
-      else
-        llvm_unreachable("Expected to find Decl or Stmt containing ancestor");
 
-      if (!expandsFrom(Loc, MacroLoc)) {
-        Result = Parent;
-        return true;
+      // TypeLoc and NestedNameSpecifierLoc are members of the parent map. Skip
+      // them and keep going up.
+      if (Loc.isValid()) {
+        if (!expandsFrom(Loc, MacroLoc)) {
+          Result = Parent;
+          return true;
+        }
       }
       Start = Parent;
     }

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits