vsavchenko marked an inline comment as done.
vsavchenko 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:
> > > 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`.


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

Reply via email to