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