NoQ added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:198
+ // 'self' variable of the current class method.
+ if (ReceiverSVal == Message.getSelfSVal()) {
+ // In this case, we should return the type of the enclosing class
----------------
NoQ wrote:
> vsavchenko wrote:
> > NoQ wrote:
> > > vsavchenko wrote:
> > > > NoQ wrote:
> > > > > I believe this is pretty much always the case. At least whenever
> > > > > `getInstanceReceiver()` is available. Another exception seem to be
> > > > > when `ReceiverSVal` is an `UnknownVal` (in this case `self` is going
> > > > > to be `SymbolRegionValue` because it's never set in the Store), but
> > > > > that's it. I inferred this by looking at
> > > > > `ObjCMethodCall::getInitialStackFrameContents()`.
> > > > >
> > > > > I think we should have used `getSelfSVal()` to begin with.
> > > > > I believe this is pretty much always the case.
> > > >
> > > > I didn't quite get what you mean by that
> > > >
> > > >
> > > What i'm trying to say is that `C.getSVal(RecE)` and
> > > `Message.getSelfSVal()` and `Message.getReceiverSVal()` are basically the
> > > same `SVal`. It shouldn't be necessary to check both or check whether
> > > they're the same; you must have meant to check for something else,
> > > probably something purely syntactic.
> > >
> > > ----
> > >
> > > > I inferred this by looking at
> > > > ObjCMethodCall::getInitialStackFrameContents().
> > >
> > > Wait, so it's only true for inlined methods. For non-inlined methods
> > > `getSelfSVal()` will be unknown :/
> > Yeah, that might be a bit extraneous to do it with `SVal`s, but this code
> > for sure does its job (it is definitely not a redundant check).
> > `getSelfSVal()` returns receiver of the function //containing// the call
> > and not the call itself. So, it does check if we the receiver of the
> > message is `self`.
> >
> > I changed it to this way of doing things because it is consistent with how
> > the same thing is done in `getRuntimeDefinition`.
> > `getSelfSVal()` returns receiver of the function containing the call and
> > not the call itself
>
> 😱 looks broken to me.
Let's rename `getSelfSVal()` so that it screamed that it's the callee's self as
opposed to the caller's self, and explain in a long comment why do we even care
about the caller's self. I.e., that we heuristically assume that if a class
method jumps into another class method on the same class object, it's going to
be devirtualized to the same class. Which isn't always the case, hence !Precise.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78286/new/
https://reviews.llvm.org/D78286
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
