rjmccall added inline comments.
================ Comment at: clang/lib/CodeGen/CGObjC.cpp:487 + + // Walk both lists to get the full set of implied protocols + llvm::DenseSet<const ObjCProtocolDecl *> AllImpliedProtocols; ---------------- lanza wrote: > rjmccall wrote: > > You should add something like ", including all the runtime protocols but > > not the non-runtime protocols". > Do you anticipate a usage of `getImpliedProtocols` other than this algorithm? > I include the non-runtime protos in the all implied list simply because it > simplifies the collection. e.g. you insert iff it's a runtime protocol and if > not you have to check `contains` and then potentially add a non-runtime to > the work queue as many times as it's seen. > > Ultimately their inclusion in the all-implied list is meaningless because we > never attempt to insert a non-runtime protocol into the `RuntimePDs` list > anyways. So it's either extra elements in the `AllImplied` list or extra work > in the `getImpliedProtocols` method without any behavioral differences. Ah, yes, that's a fair point. 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
