dvadym added a comment.
Thanks for comments! PTAL
Since it's added checking of trivially copyable arguments of move, it's needed
to rename this check, what do you think about MoveNoEffectCheck?
================
Comment at: clang-tidy/misc/MoveConstantArgumentCheck.cpp:11
@@ +10,3 @@
+#include "MoveConstantArgumentCheck.h"
+
+namespace clang {
----------------
aaron.ballman wrote:
> > I didn't find how it can be done, could you please advice?
>
> This is the usual way we do it (in the registerMatchers() function):
> ```
> if (!getLangOpts().CPlusPlus)
> return;
> ```
>
Thanks
================
Comment at: clang-tidy/misc/MoveConstantArgumentCheck.cpp:23
@@ +22,3 @@
+}
+
+void MoveConstantArgumentCheck::check(const MatchFinder::MatchResult &Result) {
----------------
alexfh wrote:
> The problem is that each template class or function can have several
> representations in the AST: one for the template definition and one for each
> instantiation. Usually, we don't need to even look at the instantiations,
> when we want to reason about the code in the general case. You can filter out
> expressions belonging to template instantiations using this narrowing
> matcher: `unless(isInTemplateInstantiation())`. And for template definitions
> the type will be marked as dependent.
Great, thank you. It works
================
Comment at: clang-tidy/misc/MoveConstantArgumentCheck.cpp:40
@@ +39,3 @@
+ diag(CallMove->getLocStart(), "std::move of the %select{|const "
+ "}0%select{expression|variable}1 %select{|of
"
+ "trivially-copyable type }2has no effect; "
----------------
alexfh wrote:
> dvadym wrote:
> > Could you please advice how can I correctly make removal?
> > I expected that
> > FixItHint::CreateRemoval(SourceRange(CallMove->getLocStart(),
> > Arg->getLocStart())) removes "std::move(" but it removes
> > "std::move(varname", so from a "move" call only ")" is left
> `FixItHint::CreateRemoval` and many other methods accept `CharSourceRange`
> instead of `SourceRange`. The former is a `SourceRange` + a flag telling
> whether the range should be treated as a character range or a token range. By
> default, `SourceRange` is converted to a `CharSourceRange` marked as a token
> range. So your current code creates a `FixItHint` that removes tokens from
> `std` to `varname` inclusive. If you want to delete everything from `std` to
> just before `varname`, you can create a character range from
> `CallMove->getLocStart()` to `Arg->getLocStart().getLocWithOffset(-1)`.
>
> However, when there's something between `std::move(` and `varname`
> (whitespace and/or comment(s)), might want to delete just `std::move(`. In
> this case you can take `CallMove->getCallee()' (which will correspond to
> `std::move`), and then find the first '(' token after it's end location. It's
> probably a rare case though, so let's go for the simpler solution for now
> (with `getLocWithOffset` and character ranges).
Thanks, creating CharSourceRange makes the trick. Also as we talked offline
I've added a call of Lexer::makeFileCharRange( for processing move call in
macros. Please have a look.
http://reviews.llvm.org/D12031
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
