aaron.ballman added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp:66
+bool isPassedToStdMove(const ParmVarDecl &Param, ASTContext &Context) {
+ // Check if the parameter has a name, in case of functions like -
----------------
Can `Context` be `const ASTContext &`?
================
Comment at:
clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp:74
+ // parmVarDecl picked up by this checker. It will be an empty string and will
+ // lead to an assertion failure when using hasName(std::string) being used
+ // in the matcher below. If empty then exit indicating no move calls present
----------------
This may be a bigger issue with `hasName()` as this strikes me as possibly
being a bug. I would expect `hasName("")` to return `false` for any AST node
which has a nonempty name, and `true` for any AST node without a name.
================
Comment at:
clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp:77
+ // for the function argument being examined.
+ const auto ParamName = Param.getName();
+
----------------
We don't typically use top-level `const` in the project, so you can drop that,
and you should only use `auto` when the type is spelled out in the
initialization (or is impossible to spell, or strikingly obvious from context
like iterators), so you should spell that type out in this case.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87540/new/
https://reviews.llvm.org/D87540
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
