[PATCH] D92797: APINotes: add initial stub of APINotesWriter
Saleem Abdulrasool via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 19 14:32:46 PST 2021
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
More information about the cfe-commits
mailing list