[PATCH] D12031: Const std::move() argument ClangTidy check

Vadym Doroshenko via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 24 10:22:28 PST 2015


dvadym added a comment.

Thanks alexfh! I've addressed your comments and uploaded new patch. PTAL


================
Comment at: clang-tidy/misc/MoveConstantArgumentCheck.cpp:56
@@ +55,3 @@
+        << IsConstArg << IsVariable << IsTriviallyCopyable
+        << FixItHint::CreateRemoval(Lexer::makeFileCharRange(
+               CharSourceRange::getCharRange(CallMove->getLocStart(),
----------------
alexfh wrote:
> After some thinking, there may be cases where the range of the whole expression can be translated to a file char range, but a sub-range of it can't. So you need to check the validity of the results of both `makeFileCharRange` calls in this expression before creating a fixit hint with the resulting ranges.
> 
> Also, it seems reasonable to issue a warning in any case, and fix-it hints only when we are rather certain that we can apply them safely (which the validity of all `makeFileCharRange` results should tell us about).
I've changed: now BeforeArgumentsRange and AfterArgumentsRange are calculated if they are valid then Removal is created. But probably it's better to write a test when some of these ranges are valid, could you please advice when it can be?


http://reviews.llvm.org/D12031





More information about the cfe-commits mailing list