SymbolSet is a structure that acts as a simple container class for exported symbols that
belong to a library interface. It allows tapi to decouple the globals
from the other library attributes. It's uniqued by symbol name and kind, which all contain their assigned target triples.
Details
- Reviewers
ributzka zixuw - Commits
- rG0882c70df222: [TextAPI] Introduce SymbolSet
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/TextAPI/SymbolSet.cpp | ||
---|---|---|
29 | Why add the corresponding Objective-C class here too? |
llvm/lib/TextAPI/SymbolSet.cpp | ||
---|---|---|
29 | hm, I'll try removing it. TAPI always expects these symbols to also have a corresponding class symbol. In theory, these should always exist so it doesn't need to be implicitly added. Though it's possible to export a ObjectiveCClassEHType without a ObjectiveCClass |
llvm/include/llvm/TextAPI/InterfaceFile.h | ||
---|---|---|
346 | nit: why do we need an std::optional instead of using the nullptr to indicate non-existence? And if the method returns the query element maybe we should give it another name than contains? |
llvm/include/llvm/TextAPI/InterfaceFile.h | ||
---|---|---|
346 | nullptr and std::nullopt evaluate differently in e.g. if (auto x = file->contains(sym)). I don't feel that strongly about supporting but it is used that way downstream. | |
llvm/include/llvm/TextAPI/Symbol.h | ||
73–75 | This is just moved from InterfaceFile.cpp, I realized I forgot to delete that in this patch (will fix that) but I'd rather not update this code unless theres a strong reason to since its used in a lot of tapi data structure operations. Maybe worth revisiting though, if the code can be cleaned up. | |
llvm/lib/TextAPI/Symbol.cpp | ||
68 | I needed it to pass tests. This was an attempt to still check a TBD-v5 only attribute against older TBD file formats. Its not very useful or sound so I just removed it. The lambda should be removed too though. |
llvm/lib/TextAPI/Symbol.cpp | ||
---|---|---|
68 | NVM, I meant the removal is valid to be able to compare older files. Just trying to conditionalize it based off indirect assumptions isn't useful ATM. To be able to do this accurately I'd need to pass FileType down to this comparator. |
nit: why do we need an std::optional instead of using the nullptr to indicate non-existence? And if the method returns the query element maybe we should give it another name than contains?