[PATCH] D92797: APINotes: add initial stub of APINotesWriter

Gabor Marton via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 19 06:28:26 PST 2021


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



More information about the cfe-commits mailing list