Febbe added a comment.
So I finally had time to apply your feedback.
================
Comment at:
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyOnLastUseCheck.cpp:141
+ assert(BlockElement.DeclRefBlock);
+ auto Block = BlockElement.DeclRefBlock->succs();
+ // No successing DeclRefExpr found, appending successors
----------------
njames93 wrote:
> replace auto with type here.
Unused, because inlined next line.
================
Comment at:
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyOnLastUseCheck.cpp:236
+ for (auto &Type : BlockedTypes) {
+ if (Param->getType().getAsString().find(Type) != std::string::npos) {
+ return; // The type is blocked
----------------
njames93 wrote:
> Isn't blocked types meant to be a regular expression?
It is also handled twice, therefor I just removed it.
================
Comment at:
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyOnLastUseCheck.cpp:294
+ }
+ // Generate Diagnostic
+}
----------------
njames93 wrote:
> What's this comment for?
Removed, old Todo I have overseen.
================
Comment at:
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyOnLastUseCheck.h:10
+
+#pragma once
+#ifndef
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_UNNECESSARY_COPY_ON_LAST_USE_H
----------------
njames93 wrote:
> Remove this, the include guard is enough.
Yes, I thought, `#pragma once` is less bug prone and might be faster.
================
Comment at:
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyOnLastUseCheck.h:21
+namespace clang {
+namespace tidy::performance {
+
----------------
njames93 wrote:
> I have no preference either way, but can these namespaces all be nested or
> none nested.
Had a forward declaration there, and added it again to reduce include
dependencies.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-on-last-use.cpp:3
+// RUN: %check_clang_tidy %s -std=c++11
performance-unnecessary-copy-on-last-use %t
+#include <type_traits>
+// CHECK-FIXES: #include <utility>
----------------
njames93 wrote:
> We avoid using the standard library for tests, if you need to use anything
> from it you have to reimplement the parts you need yourself.
Kept the `static_assert` as comment and removed the header.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137205/new/
https://reviews.llvm.org/D137205
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
