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

[Michael Wyman via Phabricator via cfe-commits]

mwyman marked 2 inline comments as done.
mwyman added inline comments.


================
Comment at: clang/lib/CodeGen/CGObjC.cpp:1125
+    llvm::Type *selType = CGF.ConvertType(CGF.getContext().getObjCSelType());
+    return llvm::UndefValue::get(selType);
+  }
----------------
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.


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

https://reviews.llvm.org/D135091