[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>`.


https://reviews.llvm.org/D26040





More information about the llvm-commits mailing list