aaron.ballman added a subscriber: klimek.
================
Comment at: clang-tidy/misc/MoveForwardingReferenceCheck.cpp:44-54
@@ +43,13 @@
+ if (OriginalText == "::std::move") {
+ Diag << FixItHint::CreateReplacement(CallRange, "::std::" + ForwardName);
+ // If the original text was simply "move", we conservatively still put a
+ // "std::" in front of the "forward". Rationale: Even if the code
+ // apparently had a "using std::move;", that doesn't tell us whether it
+ // also had a "using std::forward;".
+ } else if (OriginalText == "std::move" || OriginalText == "move") {
+ Diag << FixItHint::CreateReplacement(CallRange, "std::" + ForwardName);
+ }
+ }
+}
+
+// Returns whether the UnresolvedLookupExpr (potentially) refers to
std::move().
----------------
I kind of thought that was going to be the case, but wanted to double-check. I
agree with the notion, but disagree with the approach -- as @etienneb mentions,
this code is not resilient enough to actually do the right thing. I don't think
we should be going back to the original source text, but should instead be
using the information the AST already tracks. This will perform better and do
the right thing in all cases.
================
Comment at: clang-tidy/misc/MoveForwardingReferenceCheck.cpp:93
@@ +92,3 @@
+ hasArgument(0, ignoringParenImpCasts(declRefExpr(
+ to(ForwardingReferenceParmMatcher)))))
+ .bind("call-move"),
----------------
I wonder if there's a reason for this behavior, or if it's simple a matter of
not being needed previously and so it was never implemented. @sbenza or @klimek
may know. I think we should be fixing the RecursiveASTVisitor, unless there is
a valid reason not to (which there may be), though that would be a separate
patch (and can happen after we land this one).
https://reviews.llvm.org/D22220
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
