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

Manuel Klimek klimek at google.com
Fri Oct 3 06:22:10 PDT 2014


Overall, I think Anything and Nothing are not intuitive names for me here... Perhaps more docs will make it clear what they should be called.

================
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);
+
----------------
a) why return anything()? (document the decisions, seems kinda arbitrary)
b) typo: Ancestor

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

================
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" },
----------------
I just realize this all might need some documentation.

================
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"))))
----------------
Is this test change related?

http://reviews.llvm.org/D5580






More information about the cfe-commits mailing list