Febbe added a comment.

In D137205#3912243 <https://reviews.llvm.org/D137205#3912243>, @Skylion007 
wrote:

> One other bug I found with this diff, is it seems to suggest calling 
> std::move() on function args that are references, despite the fact that 
> invalidating the reference to the input arg could be undesirable. For 
> instance take a function that takes in a reference to a String, assigns a new 
> value to the arg reference, and then returns the value of the reference. It 
> suggests calling std::move() on the arg ref when it is returned, which 
> invalidate the reference to the argument.

Oh, that's not good. Actually, it should not run on references, only  
`declRefExpr(hasType(qualType(unless(anyOf(

  isConstQualified(), referenceType(), pointerType() ,..)` are allowed to be 
parsed.

Did you run this check alone, without other checks?

Regarding the appearance of `std::move(std::move( var );`, was there a move 
already? Does it occur in a header file? In case of running clang-tidy with 
fixups in parallel over multiple sources, it can happen, that a header file is 
parsed twice at the same time, resulting in duplicate insertions.

And last but not least, can you provide a source snipped or AST for both 
appearances?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137205/new/

https://reviews.llvm.org/D137205

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to