[PATCH] D29362: Return Error instead of bool from mergeTypeStreams().

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/CodeView/TypeStreamMerger.h:16
 #include "llvm/DebugInfo/CodeView/TypeTableBuilder.h"
 
 namespace llvm {
----------------
Should probably `#include "llvm/Support/Error.h"` here.


================
Comment at: llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp:98-99
+    if (!Record.remapTypeIndices(IndexMap))
+      LastError =
+          llvm::make_error<CodeViewError>(cv_error_code::corrupt_record);
     FieldListBuilder.writeMemberType(Record);
----------------
If there is more than one error, this will trigger an exception because you will have not checked the previous value of the error.  Instead of storing an `Optional<Error>` maybe try storing just an `Error`, and then saying 

```
LastError = llvm::join_errors(std::move(LastError), llvm::make_error<CodeViewError>(cv_error_code::corrupt_record));
```


================
Comment at: llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp:180
+
+  if (LastError.hasValue()) {
+    Error E = std::move(*LastError);
----------------
If you do the above, you can simply return `LastError` here.


https://reviews.llvm.org/D29362