lanza requested review of this revision. lanza marked an inline comment as done. lanza added inline comments.
================ Comment at: clang/lib/CodeGen/CGObjC.cpp:490 + llvm::UniqueVector<ObjCProtocolDecl *> FoundProtocols; + std::set<ObjCProtocolDecl *> DeclaredProtocols; + ---------------- rjmccall wrote: > 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). Got ya, that should be much better for the 99.99% case where there are no non-runtime protocols. 👍 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75574/new/ https://reviews.llvm.org/D75574 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
