[PATCH] Fix bug in DynTypedMatcher::constructVariadic() that would cause false negatives.

Samuel Benzaquen sbenza at google.com
Fri Oct 3 07:06:26 PDT 2014


Reverted a lot of the change.
Please review against the base.

================
Comment at: include/clang/AST/ASTTypeTraits.h:94-98
@@ +93,7 @@
+
+  /// \brief Return the most derived common anscestor between Kind1 and Kind2.
+  ///
+  /// Return anything() if they are not related.
+  static ASTNodeKind getMostDerivedCommonAnscestor(ASTNodeKind Kind1,
+                                                   ASTNodeKind Kind2);
+
----------------
klimek wrote:
> a) why return anything()? (document the decisions, seems kinda arbitrary)
> b) typo: Ancestor
Removed the concept of 'anything'. was not necessary and the name was not correct.
Fixed the typo everywhere (I think).

================
Comment at: include/clang/AST/ASTTypeTraits.h:105-106
@@ -83,3 +104,4 @@
   enum NodeKindId {
-    NKI_None,
+    NKI_Nothing,
+    NKI_Anything,
     NKI_CXXCtorInitializer,
----------------
klimek wrote:
> Can you document what Nothing and Anything is in NodeKind land, and document that?
Reverted this.

================
Comment at: lib/AST/ASTTypeTraits.cpp:23-32
@@ -22,11 +22,12 @@
 
 const ASTNodeKind::KindInfo ASTNodeKind::AllKindInfo[] = {
-  { NKI_None, "<None>" },
-  { NKI_None, "CXXCtorInitializer" },
-  { NKI_None, "TemplateArgument" },
-  { NKI_None, "NestedNameSpecifier" },
-  { NKI_None, "NestedNameSpecifierLoc" },
-  { NKI_None, "QualType" },
-  { NKI_None, "TypeLoc" },
-  { NKI_None, "Decl" },
+  { NKI_Nothing, "<Nothing>" },
+  { NKI_Anything, "<Anything>" },
+  { NKI_Anything, "CXXCtorInitializer" },
+  { NKI_Anything, "TemplateArgument" },
+  { NKI_Anything, "NestedNameSpecifier" },
+  { NKI_Anything, "NestedNameSpecifierLoc" },
+  { NKI_Anything, "QualType" },
+  { NKI_Anything, "TypeLoc" },
+  { NKI_Anything, "Decl" },
 #define DECL(DERIVED, BASE) { NKI_##BASE, #DERIVED "Decl" },
----------------
klimek wrote:
> I just realize this all might need some documentation.
Reverted this.

================
Comment at: unittests/ASTMatchers/Dynamic/RegistryTest.cpp:350
@@ -349,3 +349,3 @@
                        constructMatcher("hasName", std::string("Foo"))),
-      constructMatcher("namedDecl",
+      constructMatcher("functionDecl",
                        constructMatcher("hasName", std::string("foo"))))
----------------
klimek wrote:
> Is this test change related?
> 
Yes. The change triggered the bug.
recordDecl is a namedDecl, so it wasn't hitting the bug. I made it more specific with functionDecl and it started failing. The fix made it pass again.

http://reviews.llvm.org/D5580






More information about the cfe-commits mailing list