steakhal added a comment.
Please, consider stating the motivation behind this change.
For me, by looking at the modified test, it feels like we would lose legitimate
findings (aka. true-positive reports) by applying this change.
From my perspective, the store to `i` is //dead//, since it will be immediately
overwritten in the `referenceParameters()` function.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp:536-537
+ void findReferenceParameter(const FunctionDecl *FD, const Expr *const *Args)
{
+ unsigned numParams = FD->getNumParams();
+ for (unsigned i = 0; i < numParams; ++i) {
+ if (FD->getParamDecl(i)->getType()->isReferenceType()) {
----------------
You don't seem to use the index, only for iterating over the parameters.
I believe, `FD->parameters()` would simplify this code.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp:538-545
+ if (FD->getParamDecl(i)->getType()->isReferenceType()) {
+ if (auto *DRE = dyn_cast<DeclRefExpr>(Args[i])) {
+ if (auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
+ Escaped.insert(VD);
+ }
+ }
+ }
----------------
By looking at e.g. `findLambdaReferenceCaptures()`, it seems like we should
prefer `continue` statements for filtering; probably to avoid such deeply
nested intendations.
Along with that, I think the llvm style guide prefers not wrapping single
statement branches by courly braces.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp:549
+ void findCallReferenceParameters(const CallExpr *CE) {
+ if (auto *FD = dyn_cast<FunctionDecl>(CE->getCalleeDecl())) {
+ findReferenceParameter(FD, CE->getArgs());
----------------
I'm not sure `getCalleeDecl()` always returns a valid pointer.
I believe the `dyn_cast_or_null` would be more appropriate in this case.
This theory is underpinned by the other uses of the `getCalleeDecl()` API.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp:555
+ void findConstructorReferenceParameters(const CXXConstructExpr *CE) {
+ findReferenceParameter(CE->getConstructor(), CE->getArgs());
+ }
----------------
Why don't you use the `CE->arguments()` method instead? That would result in a
range (begin + end).
I believe that is superior in most cases.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131067/new/
https://reviews.llvm.org/D131067
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
