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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
