martong added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:92-125
+class MakeRetVal {
+ const CallExpr *CE = nullptr;
+ std::unique_ptr<DefinedSVal> RetVal;
+ SymbolRef RetSym;
+
+public:
+ MakeRetVal(const CallEvent &Call, CheckerContext &C)
----------------
Szelethus wrote:
> balazske wrote:
> > Szelethus wrote:
> > > balazske wrote:
> > > > Szelethus wrote:
> > > > > Do you have other patches that really crave the need for this class?
> > > > > Why isn't `CallEvent::getReturnValue` sufficient? This is a
> > > > > legitimate question, I really don't know. :)
> > > > This is an "interesting" solution for the problem that there is need
> > > > for a function with 3 return values. The constructor performs the task
> > > > of the function: Create a conjured value (and get the various objects
> > > > for it). The output values are RetVal and RetSym, and the success
> > > > state, and the call expr that is computed here anyway. It could be
> > > > computed independently but if the value was retrieved once it is better
> > > > to store it for later use. (I did not check how costly that operation
> > > > is.)
> > > >
> > > > I had some experience that using only `getReturnValue` and make
> > > > constraints on that does not work as intended, and the reason can be
> > > > that we need to bind a value for the call expr otherwise it is an
> > > > unknown (undefined?) value (and not the conjured symbol)?
> > > I suspect that `getReturnValue` might only work in `postCall`, but I'm
> > > not sure.
> > >
> > > I think instead of this class, a function returning a `std::tuple` would
> > > be nicer, with `std::tie` on the call site. You seem to use all 3 returns
> > > values in the functions that instantiate `MakeRetVal` anyways :).
> > >
> > > In `StdLibraryFunctionsChecker`'s `evalCall`, the return value is
> > > [[https://github.com/llvm/llvm-project/blob/master/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp#L403|explicitly
> > > constructed]], and further constraints on it are only imposed in
> > > `postCall`. I wonder why that is. @martong, any idea why we don't `apply`
> > > the constraints for pure functions in `evalCall?`
> > The return value case is not as simple because the `DefinedSVal` has no
> > default constructor, but it is not as bad to return only the `RetVal` and
> > have a `CE` argument.
> I like the current solution very much!
> In StdLibraryFunctionsChecker's evalCall, the return value is explicitly
> constructed, and further constraints on it are only imposed in postCall. I
> wonder why that is. @martong, any idea why we don't apply the constraints for
> pure functions in evalCall?
We could apply them in evalCall technically.
I think the reason why we don't do that is the matter of implementation, and
more importantly this way we are consequent with the traditional Hoare logic:
{Pre}C{Post} as {Post} is done in postCall.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75356/new/
https://reviews.llvm.org/D75356
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
