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

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/lib/CodeGen/CGObjC.cpp:490
+  llvm::UniqueVector<ObjCProtocolDecl *> FoundProtocols;
+  std::set<ObjCProtocolDecl *> DeclaredProtocols;
+
----------------
You should use llvm::DenseSet for this.  But in general there are more sets here than I think you really need, and you're building them eagerly when you don't have to.  I would suggest:

1. Walk the list of declared protocols, adding the runtime protocols to the main result list and the first runtime protocols implied by the non-runtime protocols to a different list.

2. If there are any protocols in the first-implied list (which is unlikely), build the implied-protocols set for all the protocols in the main list (inclusive of the protocols there) and the first-implied list (exclusive).  It would be totally reasonable to add a method to make this easier: `void ObjCProtocolDecl::getImpliedProtocols(llvm::DenseSet<const ObjCProtocolDecl*> &) const;`

3. Add any protocols that are in the first-implied list but not the implied set back to the main list.

Also, protocols can be redeclared, so you should make sure to map to the canonical decl before adding them to a set (or UniqueVector).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574