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



_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to