aaron.ballman added a comment.
I have some quick drive-by comments but still have to think about the test
cases a bit more.
================
Comment at:
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:74
+// the BlockStmt. It does this by checking the following:
+// 1. If the variable is neither a reference nor a pointer then the the
+// isOnlyUsedAsConst() check is sufficient.
----------------
typo: the the
================
Comment at:
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:77
+// 2. If the (reference or pointer) variable is not initialized in a DeclStmt
in
+// the BlockStmt. In this case it its pointee is likely not modified (unless it
+// is passed as an alias into the method as well).
----------------
typo: it its
================
Comment at:
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:83
+// object arg or variable that is referenced is immutable as well.
+bool isInitializingVariableImmutable(const VarDecl &InitializingVar,
+ const Stmt &BlockStmt,
----------------
You should mark the function as being `static`.
================
Comment at:
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:86
+ ASTContext &Context) {
+ if (!isOnlyUsedAsConst(InitializingVar, BlockStmt, Context)) {
+ return false;
----------------
Elide braces around single-line `if` statements. For the cases involving
comments, I'd recommend hoisting the comments above the if when returning a
constant.
================
Comment at:
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:90
+ QualType T = InitializingVar.getType();
+ if (!llvm::dyn_cast<ReferenceType>(T) && !llvm::dyn_cast<PointerType>(T)) {
+ // The variable is a value type and we know it is only used as const. Safe
----------------
No need for the `llvm::`, is there? This should be replaced with
`!isa<ReferenceType, PointerType>(T)` since you only care about the Boolean
results.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91893/new/
https://reviews.llvm.org/D91893
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
