Charusso added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:98
+ SVal RetV = CE.getReturnValue();
+ Optional<DefinedOrUnknownSVal> RetDV = RetV.getAs<DefinedOrUnknownSVal>();
+
----------------
NoQ wrote:
> `castAs` if you're sure.
>
> Generally it's either evaluated conservatively (so the return value is a
> conjured symbol) or inlined (and in this case returning an undefined value
> would result in a fatal warning), so return values shouldn't ever be
> undefined.
>
> The only way to obtain an undefined return value is by binding it in
> `evalCall()`, but even then, i'd rather issue a warning immediately.
Sometimes it failed with live tests in the wild.
================
Comment at: clang/test/Analysis/return-value-guaranteed.cpp:20
+struct Foo { int Field; };
+bool error();
+bool problem();
----------------
NoQ wrote:
> NoQ wrote:
> > `core.builtin.NoReturnFunctions` reacts on this function. You can easily
> > see it in the explodedgraph dump, as the last sink node in
> > `test_calls::parseFoo` is tagged with that checker. You'll have to pick a
> > different name for testing purposes.
> (yes, this is why your tests aren't working)
> (see also D63965)
I was not sure why do you mention an unrelated patch, but I realized as the
previous graph-rewriter patches, it has my stuff in the summary. That would be
the last thing I considered. It is a nice catch, thanks!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63915/new/
https://reviews.llvm.org/D63915
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
