[PATCH] D135091: Load the `_cmd` selector for generated getters/setters of `direct` Objective-C properties.

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore accepted this revision.
stephanemoore added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/lib/CodeGen/CGObjC.cpp:1189-1191
     // Return (ivar-type) objc_getProperty((id) self, _cmd, offset, true).
     // FIXME: Can't this be simpler? This might even be worse than the
     // corresponding gcc code.
----------------
I couldn't immediate tell if this TODO is still in the best position. I think it's fine?


================
Comment at: clang/test/CodeGenObjC/direct-method.m:171-177
+// Check the synthesized objectProperty calls objc_getProperty(); this also
+// checks that the synthesized method accesses _cmd (or rather loads the
+// selector, as it is an argument to objc_getProperty).
+// CHECK-LABEL: define hidden i8* @"\01-[Root objectProperty]"(
+// CHECK-LABEL: objc_direct_method.cont:
+// CHECK-NEXT: load i8*, i8** @OBJC_SELECTOR_REFERENCES_
+// CHECK: call i8* @objc_getProperty({{.*}})
----------------
I'm not intricately familiar with the codegen checking mechanism in these tests but without fully knowing the details, this seems consistent with the checks in `-[Root accessCmd]` above which I would expect to require similar checks.

Did you check that these added tests reproduce the previous assertion/failure so that we can be confident that the code changes resolve the issue?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135091