flx added a comment.

Thanks for improving this check!



================
Comment at: 
clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp:177-178
     const auto &CurrentParam = *FunctionDecl->getParamDecl(Index);
+    if (IsExplicitTemplateSpecialization && Function != FunctionDecl)
+      continue;
     Diag << utils::fixit::changeVarDeclToReference(CurrentParam,
----------------
Could you add a comment here why we're skipping the fix here?


================
Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param.cpp:388
+  // CHECK-MESSAGES: [[@LINE-1]]:54: warning: the parameter 'E' is copied
+  // CHECK-FIXES: T templateSpecializationFunction(const ExpensiveToCopyType& 
E) {
+  return T();
----------------
Should we apply the fixes or just issue the warning? For virtual methods we 
suppress the fix since we can't necessarily update all overrides of the method. 
Are template specializations always guaranteed to be in the same translation 
unit which  would make this safe?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116593

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

Reply via email to