vsavchenko added a comment.
Looks like a solid start!
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:301
+
+ bool addTransition = false;
+ ProgramStateRef State = C.getState();
----------------
nit: variable names should be capitalized
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:321-323
+ State =
+ State->assume(llvm::dyn_cast<DefinedOrUnknownSVal>(RetVal), true);
+ addTransition = true;
----------------
Another idea here.
Instead of repeating:
```
State = State->assume(llvm::dyn_cast<DefinedOrUnknownSVal>(RetVal), VALUE);
addTransition = true;
```
and having boolean `addTransition`, you can have `Optional<bool> CompResult`
and do:
```
CompResult = VALUE;
```
And do the actual assumption at the bottom section when `CompResult.hasValue()`
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:334
+ default:
+ assert(false && "shouldn't reach here");
+ }
----------------
nit: `llvm_unreachable` instead of `assert(false)`
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:345-364
+ if (FirstPtr && !SecondPtr &&
+ State->assume(llvm::dyn_cast<DefinedOrUnknownSVal>(*FirstPtr),
+ false)) {
+ // FirstPtr is null, SecondPtr is unknown
+ if (OOK == OO_LessEqual) {
+ State =
+ State->assume(llvm::dyn_cast<DefinedOrUnknownSVal>(RetVal),
true);
----------------
These are also symmetrical.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:373-391
+ switch (OOK) {
+ case OO_Equal:
+ case OO_GreaterEqual:
+ case OO_LessEqual:
+ State = State->assume(llvm::dyn_cast<DefinedOrUnknownSVal>(RetVal),
+ true);
+ addTransition = true;
----------------
This switch exactly repeats the previous switch.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:394-436
+ if (FirstNull && !SecondNull) {
+ switch (OOK) {
+ case OO_Less:
+ case OO_LessEqual:
+ State = State->assume(llvm::dyn_cast<DefinedOrUnknownSVal>(RetVal),
+ true);
+ addTransition = true;
----------------
These are symmetrical, is there a way to implement it without this boilerplate?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104616/new/
https://reviews.llvm.org/D104616
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
