[PATCH] D33564: [CodeView Type Merging] Make a Type Index Iterator that bypasses the visitor framework

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/CodeView/TypeIndexIterator.h:1
+//===- TypeIndexIterator.h --------------------------------------*- C++ -*-===//
+//
----------------
rnk wrote:
> This basically builds a SmallVector. Its... sort of like an iterator. Do we still want to call it one? I feel like "Iterator" has a specific meaning in C++, and this isn't it. Maybe `TypeIndexFinder.h` to go with `findTypeIndexOffsets` or something?
> 
> Alternatively, do you want to just mush all this functionality into `TypeSerializer.h/cpp`?
Yea, I originally intended to give it an iterator like interface, but it was non-trivial and this was just easier.  And then I forgot to change the file name.  Anyway, yea I'll change it.


================
Comment at: llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h:288
+                PointerOptions PO, uint8_t Size,
+                TypeIndex MemberPtrContainingType)
       : TypeRecord(TypeRecordKind::Pointer), ReferentType(ReferentType),
----------------
rnk wrote:
> How is this new constructor used? It's reasonable to not force the caller to construct a MemberPointerInfo and instead take both the Class type index and the member pointer representation, but defaulting unknown seems error-prone.
It's used in the unit test.


https://reviews.llvm.org/D33564