xazax.hun added inline comments.
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:435
+ void markNotInteresting(SymbolRef sym);
+
----------------
Bikeshedding: I wonder if we prefer `Uninteresting` to `NotInteresting`. Or
alternatively, if we want to emphasize this is only the lack of
interestingness, we could name it `removeInterestingness`. I do not have strong
feelings about any of the options, I was just wondering if any of you has a
preference.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:191
+ // Check the first arg, if it is of std::unique_ptr type.
+ assert(Call.getNumArgs() == 2 && "std::swap should have two arguments");
+ const Expr *FirstArg = Call.getArgExpr(0);
----------------
I wonder about the value of this assertion. Shouldn`t
`Call.isCalled(StdSwapCall)` already validate the number of arguments?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:193
+ const Expr *FirstArg = Call.getArgExpr(0);
+ if (!smartptr::isStdSmartPtr(FirstArg->getType()->getAsCXXRecordDecl())) {
+ return false;
----------------
Nit: we usually omit braces for simple `if` statements.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:458-467
+ if (BR.isInteresting(FirstThisRegion) &&
+ !BR.isInteresting(SecondThisRegion)) {
+ BR.markInteresting(SecondThisRegion);
+ BR.markNotInteresting(FirstThisRegion);
+ }
+ if (BR.isInteresting(SecondThisRegion) &&
+ !BR.isInteresting(FirstThisRegion)) {
----------------
vsavchenko wrote:
> nit: these two pieces of code are very much the same
I guess we could make `markInteresting` take an optional bool and swap the
interestingness unconditionally.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104300/new/
https://reviews.llvm.org/D104300
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
