[PATCH] D35496: [codeview] Don't use the type visitor to merge types

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 17 12:22:21 PDT 2017


ruiu accepted this revision.
ruiu added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp:282
+  discoverTypeIndices(Type.RecordData, Refs);
+  bool Success = remapIndices(R, Refs);
+  if (isIdRecord(Type.kind())) {
----------------
rnk wrote:
> ruiu wrote:
> > What is the meaning of this boolean varaible? Looks like you are not using its value.
> It indicates if all type indices were successfully remapped. This gets passed into writeIdRecord / writeRecord, and then we decide to either write the record or map this index to LF_NOTTRANSLATED. It could be renamed AllIndicesMapped, or something.
AllIndicesMapped would be a good name because I expected you would eventually return `Succcess` from this function as you did below.


================
Comment at: llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp:299
+                                    ArrayRef<TiReference> Refs) {
+  auto OriginalData = Record.OriginalRecord.content();
+  bool Success = true;
----------------
Can you replace `auto` with a concrete type?


https://reviews.llvm.org/D35496





More information about the llvm-commits mailing list