[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