xazax.hun added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:329
+ return *Ptr;
+ return C.getSValBuilder().conjureSymbolVal(
+ E, C.getLocationContext(),
----------------
In case we conjure a new symbol, we want this stored in the `TrackedRegionMap`.
So the analyzer can correctly record the constraints between the two symbols.
================
Comment at: clang/test/Analysis/smart-ptr.cpp:466
+
+ clang_analyzer_eval(ptr == ptr); // expected-warning{{TRUE}}
+ clang_analyzer_eval(ptr > ptr); // expected-warning{{FALSE}}
----------------
RedDocMD wrote:
> xazax.hun wrote:
> > Putting tests like this on the same path can be risky. Tests might split
> > the execution path (maybe not now, but in the future). I think it might be
> > more future proof to have a large switch statement that switches on an
> > unknown value and put the tests in separate cases.
> I didn't quite get you.
You remember this in the other patch:
```
member-constructor.cpp:15:5: warning: FALSE [debug.ExprInspection]
clang_analyzer_eval(*P->p == 0);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
member-constructor.cpp:15:25: note: Assuming the condition is false
clang_analyzer_eval(*P->p == 0);
^~~~~~~~~~
member-constructor.cpp:15:5: note: FALSE
clang_analyzer_eval(*P->p == 0);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
member-constructor.cpp:15:5: warning: TRUE [debug.ExprInspection]
clang_analyzer_eval(*P->p == 0);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
member-constructor.cpp:15:25: note: Assuming the condition is true
clang_analyzer_eval(*P->p == 0);
^~~~~~~~~~
member-constructor.cpp:15:5: note: TRUE
clang_analyzer_eval(*P->p == 0);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 warnings generated.
```
It looks like this does not happen for overloaded comparison operators at the
moment. But we might want to do that in the future (@NoW what do you think). I
was wondering, if we want to future proof these test cases for that behavior.
But looking at the test cases again, you only have two, where the expected
result is unknown, and they are at the very end. So feel free to ignore this
and leave the code as is.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104616/new/
https://reviews.llvm.org/D104616
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
