compnerd added a comment.

Welcome back, hope you had a good time off.  Thanks for the review!



================
Comment at: clang/lib/APINotes/APINotesWriter.cpp:35
+  bool SwiftImportAsMember = false;
+#endif
+
----------------
martong wrote:
> 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?
Yeah, this is not meant to be committed with that in place, I just need to keep 
that around for testing locally.  I will absolutely be removing them.


================
Comment at: clang/lib/APINotes/APINotesWriter.cpp:764
+
+  hash_value_type ComputeHash(key_type_ref Key) {
+    return llvm::DenseMapInfo<StoredObjCSelector>::getHashValue(Key);
----------------
martong wrote:
> Some other *TableInfo classes do not have `ComputeHash`. E.g. 
> `GlobalVariableTableInfo`. Why?
> Where do we use `ComputeHash`?
Excellent observation.  The `ComputeHash` is not directly used, it is 
implemented as part of a trait used for the hashing, used in LLVM as part of 
`llvm::OnDiskChainedHashTableGenerator` which is used throughout this file.  
The reason that `GlobalVariableTableInfo` does not use it is because the 
versioned items derive from `VersionedInfo` which provides a shared 
implementation.


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

Reply via email to