[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Thanks, that's a lot better.  Just some minor suggestions.



================
Comment at: clang/lib/CodeGen/CGObjC.cpp:477
+  // If there are no non-runtime protocols then we can just stop now.
+  if (!NonRuntimePDs.size())
+    return RuntimePds;
----------------
`empty()`, please


================
Comment at: clang/lib/CodeGen/CGObjC.cpp:487
+
+  // Walk both lists to get the full set of implied protocols
+  llvm::DenseSet<const ObjCProtocolDecl *> AllImpliedProtocols;
----------------
You should add something like ", including all the runtime protocols but not the non-runtime protocols".


================
Comment at: clang/lib/CodeGen/CGObjC.cpp:499
+
+  for (const auto *PD : ResolvedProtos) {
+    if (!AllImpliedProtocols.contains(PD)) {
----------------
Comment on this pass.


================
Comment at: clang/lib/CodeGen/CGObjC.cpp:501
+    if (!AllImpliedProtocols.contains(PD)) {
+      const auto *Can = PD->getCanonicalDecl();
+      RuntimePds.push_back(Can);
----------------
PD is already canonical here (and if it weren't, looking in `AllImpliedProtocols` wouldn't work).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574