lanza 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;
----------------
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.


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

Reply via email to