RedDocMD marked 4 inline comments as done.
RedDocMD added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:301
+ const OverloadedOperatorKind OOK = FD->getOverloadedOperator();
+ if (!(OOK == OO_Equal || OOK == OO_ExclaimEqual || OOK == OO_Less ||
+ OOK == OO_LessEqual || OOK == OO_Greater || OOK == OO_GreaterEqual ||
----------------
xazax.hun wrote:
> So it looks like `operator<<` is the only operator we are not supporting. I
> think it might be good to include some test code to ensure that its use does
> not suppress warnings. (Also OK, if you decide to deal with this in a
> follow-up PR.)
Yes that's the next patch. (and one more for `std::hash`).
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:341
+ case OO_Spaceship:
+ // TODO: What would be a good thing to do here?
+ default:
----------------
xazax.hun wrote:
> Instead of marking this unreachable, I'd suggest to just return a conjured
> symbol for now. Usually, we should not use asserts to test user input.
Ah yes that's what is happening now. `RetVal` is initialized to a conjured
value. If we can conclude no further, then that is what is returned - which is
what happens here. In other cases, constraints are applied or it is replaced by
the output from `evalBinOp`.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:354
+
+ if (FirstPtr && !SecondPtr &&
+ State->assume(FirstPtr->castAs<DefinedOrUnknownSVal>(), false)) {
----------------
xazax.hun wrote:
> I am not sure if we actually need all this special casing. You could create
> an `SVal` that represents a nullpointer constant and use `evalBinOp` with
> that `SVal` when there is no symbol available.
Actually the naming is a bit misleading here, perhaps. `FirstPtr` is not the
inner pointer itself but a pointer to the //SVal//. So the test `FirstPtr &&
!SecondPtr` checks that we know the SVal for the inner pointer of the first arg
but we do not know that of the second arg. Using a null-pointer constant would
not help.
However, we could use a conjured value to simplify some work.
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
