This adds the skeleton for serializing out the APINotes data from the
APINotes. The writer uses a private implementation pattern to reduce
the exposed surface to just the programmatic representation of the
APINotes and not expose the details of the bitcode encoding. The format
itself is not considered stable and should only be accessed through the
APINotes Reader and Writer types.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I am terribly sorry for keeping you waiting always. Thanks for your patience.
clang/include/clang/APINotes/APINotesWriter.h | ||
---|---|---|
2 | How are we going to use this class? I mean what are the use cases to call the Writer from inside Clang? What will be the usual process of using an APINotes file? Is it like: 1) Edit `APINotesX.yaml` by hand (by a library author) 2) Serialize `APINotesX.yaml` -> `APINotesX.serialized` (Would this require a special Clang invocation?) 3) Run Clang for the usual tasks (analysis, etc) but with feeding it with `APINotesX.serialized` Would it be possible to feed the raw APINotesX.yaml to Clang and skip the whole serialize/deserialize steps? If yes, then maybe we should put more focus on that first (but maybe it's already too late since we added the serialization format and the reader). |
clang/include/clang/APINotes/APINotesWriter.h | ||
---|---|---|
2 | As to the question of the expected usage: if a user wishes to alter the behavior of a library (the library is missing nullability annotation!? I want that checking!) they add a supplement APINotes for the library, and pass that to the compiler when using the library via flags. When clang goes through and finds that the cache is not populated, it will process the API Notes and compile it. Then it apply the API Notes to the compilation as it goes through (looking up the interfaces it encounters). This class is going to be used by the APINotes Compiler, which will read the YAML input and then serialize it out to bitcode encoded format. This will occur implicitly by the compiler consulting a cache (similar in spirit to how modules are cached). If the APINotes Manager finds the cached copy that is used, otherwise the contents can be processed and emitted into the cache. This basically is similar in spirit to a PCH/PCM. I dont mind switching to focus on a different part of the API if it: The intent here is to chunk the pieces so that there is a way to process the changes logically and with testing. I was simply taking an approach to make progress, but as long as we can make progress, Im not really tied to a specific path through this. In fact, I was struggling with how to create a custom compiler to serialize, deserialize the input so that it can be verified (namely a question of how to query/compare - is the best that we can do diff against a golden master?) |
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.
clang/lib/APINotes/APINotesWriter.cpp | ||
---|---|---|
36 | Could you please remove from the patch then? | |
765 | Some other *TableInfo classes do not have ComputeHash. E.g. GlobalVariableTableInfo. Why? |
Welcome back, hope you had a good time off. Thanks for the review!
clang/lib/APINotes/APINotesWriter.cpp | ||
---|---|---|
36 | 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. | |
765 | 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. |
Apologies for pinging an old review. Would it be possible to land this to facilitate future work to upstream API notes support?
hi, this seems to be breaking out amdgpu opemp and hip buildbots
https://lab.llvm.org/staging/#/builders/247/builds/4967
How are we going to use this class? I mean what are the use cases to call the Writer from inside Clang?
I understand that the Reader might be used to read up the serialized APINotes file.
What will be the usual process of using an APINotes file? Is it like:
Would it be possible to feed the raw APINotesX.yaml to Clang and skip the whole serialize/deserialize steps? If yes, then maybe we should put more focus on that first (but maybe it's already too late since we added the serialization format and the reader).