[PATCH] Adding 4 ASTMatchers: typedefDecl, isInMainFile, isInSystemFile, isInFileMatchingName

Daniel Jasper djasper at google.com
Tue Jul 8 23:46:08 PDT 2014


================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:155
@@ +154,3 @@
+/// \endcode
+/// typedefType(isInMainFile())
+///   matches "typedef int X"
----------------
Samuel Benzaquen wrote:
> s/typedefType/typedefDecl/
so, this should be typedefDecl, not typedefType. Also, remove the "isInMainFile()" as that is confusing.

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:202
@@ +201,3 @@
+
+/// \brief Matches AST nodes that were expanded within files whose name is
+/// matching a given regex
----------------
Please indicate whether this is a partial match or a full match.

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:205
@@ +204,3 @@
+///
+/// Example matches Y but not X (matcher = recordDecl(isInSytemHeader())
+/// \code
----------------
Samuel Benzaquen wrote:
> Example is copy-pasted from above and not updated for isInFileMatchingName
Copy and paste error.

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:219
@@ +218,3 @@
+                          std::string, RegExp) {
+  assert(!RegExp.empty());
+
----------------
I think it is unnecessary, to assert here. The behavior is well-defined.

================
Comment at: include/clang/Tooling/Tooling.h:157
@@ -157,1 +156,3 @@
+                           const Twine &FileName = "input.cc",
+                           const std::vector<std::pair<std::string, std::string>> &VirtualMappedFiles= {});
 
----------------
Samuel Benzaquen wrote:
> I believe we can't still use brace init in clang. More instances of this in other files.
> See http://llvm.org/docs/CodingStandards.html#supported-c-11-language-and-library-features
Stick to 80 columns.

================
Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:4440
@@ +4439,3 @@
+      {{"/foo", "class A {};"}, {"/bar", "class B {};"}}));
+  ASSERT_DEBUG_DEATH({
+                       EXPECT_TRUE(matchesConditionally(
----------------
Samuel Benzaquen wrote:
> Comment why this dies.
This test should go away with the assert.

================
Comment at: unittests/ASTMatchers/ASTMatchersTest.h:65
@@ -65,1 +64,3 @@
+                                              llvm::StringRef CompileArg,
+                                              const std::vector<std::pair<std::string, std::string>> &VirtualMappedFiles= {}) {
   bool Found = false, DynamicFound = false;
----------------
Samuel Benzaquen wrote:
> over 80 chars. reflow the signature.
Stick to 80 columns.

http://reviews.llvm.org/D4283






More information about the cfe-commits mailing list