alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG modulo comment.
================
Comment at: clang-tidy/utils/DeclRefExprUtils.cpp:127
+ match(findAll(declRefExpr(equalsNode(&DeclRef),
+ unless(hasAncestor(stmt(anyOf(
+ forStmt(), cxxForRangeStmt(), whileStmt(),
----------------
flx wrote:
> alexfh wrote:
> > flx wrote:
> > > alexfh wrote:
> > > > How will this work with lambdas / local classes declared inside a loop?
> > > > Not sure if this case is going to happen in real code, but we'd better
> > > > be clear about the limitations of the implementation.
> > > Why would this not work? Could you give an example? The way the function
> > > is written it handles my the use case for identifying when moving the
> > > parameter is not safe, so I could also just move it into the
> > > UnnecessaryValueParamCheck.
> > I was thinking about a case where a loop this matcher finds is outside of
> > the function definition and shouldn't be considered:
> >
> > void F() {
> > for (;;) {
> > struct C {
> > void f(ExpensiveMovableType E) {
> > auto F = E;
> > }
> > };
> > }
> > }
> >
> This case is not an issue in the check as we're passing f's body statement to
> the hasLoopStmtAncestor function, so the search is scoped to f's body.
>
> If you're concerned about the case where someone calls this with F's body
> but expects it to be scoped to f I can just move this function into the check
> and make it an implementation detail.
Making it an implementation detail until other potential users appear sounds
good.
https://reviews.llvm.org/D27187
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
