martong accepted this revision. martong added a comment. This revision is now accepted and ready to land.
In D92797#2505254 <https://reviews.llvm.org/D92797#2505254>, @compnerd wrote: > Ping x 3 Sorry, just came back from holidays. Generally, I have no major concerns. My minor concern is that it is not easy to add a new Note. There are many different places to extend the code (i.e. adding a new DenseMap instance, adding two new write function prototypes and their definitions). I am not sure if this could be simpler though. ================ Comment at: clang/lib/APINotes/APINotesWriter.cpp:35 + bool SwiftImportAsMember = false; +#endif + ---------------- compnerd wrote: > Please ignore this `#if`-defed portion, it is not intended for upstream, but > I need to keep this working downstream. I will remove this before merging. Could you please remove from the patch then? ================ Comment at: clang/lib/APINotes/APINotesWriter.cpp:764 + + hash_value_type ComputeHash(key_type_ref Key) { + return llvm::DenseMapInfo<StoredObjCSelector>::getHashValue(Key); ---------------- Some other *TableInfo classes do not have `ComputeHash`. E.g. `GlobalVariableTableInfo`. Why? Where do we use `ComputeHash`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92797/new/ https://reviews.llvm.org/D92797 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
