[PATCH] D26040: [CodeView] Add CodeViewRecordIO for reading and writing, and use it for reading type records.
Zachary Turner via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 28 11:53:08 PDT 2016
zturner added a comment.
In https://reviews.llvm.org/D26040#582267, @inglorion wrote:
> I like how this gives us basically a single definition of how to map between objects and byte streams that we can then use for reading and writing. I am a bit unhappy with the repeated if (IsWriting) ... in CodeViewRecordIO. How would you feel about making two different classes - one to implement the reading and one to implement the writing?
Luckily `CodeViewRecordIO` is really low level and not intended to be the external interface to this thing. For that you use `TypeRecordMapping` (and in the future, `SymbolRecordMapping`). If we had something like `std::variant<>` this would be cleaner, but for now there's always going to be a check. We could hide it through a virtual function indirection, where you make an abstract base class with all the `map` methods, and then make a `ReadMapper` and a `WriteMapper`, and `TypeRecordMapper` just stores a `std::unique_ptr<MapperBase>`.
More information about the llvm-commits