aaron.ballman added inline comments.
================
Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:23
+AST_MATCHER(CXXStdInitializerListExpr, cxxStdInitializerListExpr) {
+ return true;
----------------
kuhar wrote:
> Prazek wrote:
> > kuhar wrote:
> > > alexfh wrote:
> > > > This should be a node matcher rather than a narrowing matcher, and it
> > > > should be placed to ASTMatchers.h, if there is no such matcher already.
> > > I wanted to backport this fix to 4.0.1 release after applying it in trunk.
> > > I thought that adding a new ASTMatcher would introduce an API change in
> > > clang, and I'm not sure if it's much welcome.
> > >
> > > Maybe it would be better to introduce a new matcher in ASTMatchers.h in
> > > trunk and make it an internal narrowing matcher in the backported patch?
> > > What would be best?
> > Why adding new matcher to 4.0.1 release is bad? Will it break anything?
> The example that I originally came up with was updating clang-tidy to 4.0.1
> while keeping clang libs 4.0.0, but since we just include all the matchers I
> think it should work fine.
>
> Here's a patch with the new matcher: https://reviews.llvm.org/D32810
> Why adding new matcher to 4.0.1 release is bad? Will it break anything?
Because it doesn't meet the criteria for doing a point release, generally
speaking.
I think a better approach is to leave this matcher as part of this patch, get
the patch submitted, then Tom can cherrypick the commit for 4.0.1. You can then
submit a second patch to hoist this matcher out to ASTMatchers.h and use it
within this check. (Basically do it in two steps.)
================
Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:24
+AST_MATCHER(CXXStdInitializerListExpr, cxxStdInitializerListExpr) {
+ return true;
+}
----------------
This looks odd to me. Can you do:
```
const internal::VariadicDynCastAllOfMatcher<Expr, CXXStdInitializerListExpr>
cxxStdInitializerListExpr;
```
I'm not certain whether this works or not, but we don't have matchers that just
return true.
https://reviews.llvm.org/D32767
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
