[PATCH] D135091: Create storage for the `_cmd` argument to the helper function for generated getters/setters of `direct` Objective-C properties.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.


================
Comment at: clang/lib/CodeGen/CGObjC.cpp:1125
+    llvm::Type *selType = CGF.ConvertType(CGF.getContext().getObjCSelType());
+    return llvm::UndefValue::get(selType);
+  }
----------------
mwyman wrote:
> nlopes wrote:
> > mwyman wrote:
> > > nlopes wrote:
> > > > Please consider using PoisonValue here instead (if appropriate). We are trying to get rid of undef.
> > > > Thank you!
> > > > A poison value is a result of an erroneous operation.
> > > 
> > > I could very well be misunderstanding, but based on this description from LangRef.html `PoisonValue` doesn't seem appropriate here; this is not "erroneous" behavior: it is just continuing the behavior prior to removing the `_cmd` implicit argument from the ABI, which was that the value was `undef` from the generated getter/setter's caller.
> > > 
> > > https://github.com/llvm/llvm-project/blob/49acab3f1408be9439a57833aa7b67678c9a05ba/clang/lib/CodeGen/CGObjCMac.cpp#L2142
> > > 
> > > Since this is (essentially) replicating that behavior, I'm not sure `PoisonValue` is what we want.
> > If the value is not used, then it can be poison.
> > If it gets used somehow then it depends on the callers. I don't know anything about ObjC to know what the right answer is here.
> > As we want to remove undef, the fewer the uses the better. Thank you!
> Fair enough. I've changed to pass poison into the helper functions.
It looks like `_cmd` is unused in both `objc_getProperty` and `objc_setProperty`, so I think this is fine.
 
https://github.com/opensource-apple/objc4/blob/master/runtime/objc-accessors.mm


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135091/new/

https://reviews.llvm.org/D135091