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

Manuel Klimek klimek at google.com
Mon Nov 10 05:27:15 PST 2014


I'll happily land the CL once llvm.org is up again

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:219
@@ +218,3 @@
+                          std::string, RegExp) {
+  assert(!RegExp.empty());
+
----------------
parallaxe wrote:
> djasper wrote:
> > I think it is unnecessary, to assert here. The behavior is well-defined.
> I took that from the existing matchesName-matcher (Line 1707 currently). Is there a difference between the matchesName-matcher and the isInFileMatchingName-matcher that makes sense to have a different behaviour regarding of empty regex? Or should that assertion also be removed from the matchesName-regex?
You also seem to have resolved this (same problem as other comment)?

Also, it's generally a good  idea to add a reply with a short comment "Done." to all comments people have made so it's clear they have been addressed.

================
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= {});
 
----------------
parallaxe wrote:
> djasper wrote:
> > sbenza 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.
> I see. So replacing the empty initializer list by const std::vector<std::pair<std::string, std::string>>() would do the thing, but repeating the nesting template definition doesn't appeal me. A typedef could improve it.
> Searching for std::vector<std::pair<std::string, std::string>>in the project brings the RemappedFile-variable in PreprocessorOptions.h up, so it would make sense to declare the type at a level where it could be also reused in PreprocessorOptions.h, wouldn't it?
> I would be lucky of some advices to this:
> - should I introduce a typedef for that?
> - where would be a good place?
> - what should it be named? (my first guess would be something like "FileMappings", but I'm honestly not so good in choosing names)
I assume this was something you didn't send off before? (Note that you have to click "Submit" after adding new comments, otherwise they will not be visible, even when you upload a new patch with arc)

http://reviews.llvm.org/D4283






More information about the cfe-commits mailing list