[PATCH] D121801: [llmv-pdbutil] Replace ExitOnError with explicit error handling.

[Alexandre Ganea via Phabricator via llvm-commits]

aganea added inline comments.


================
Comment at: llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp:455
 
-  ExitOnError Err("Unexpected error processing modules: ");
-
----------------
The user would loose the context after removal of this line, if the message is printed on stderr. Can you move this line to `L201`, or replace it by an extra `StringError` if you plan on moving this entire file to the PDB library? It could be something like:
```
  if (opts::dump::DumpSymbols) {
    auto EC = File.isPdb() ? dumpModuleSymsForPdb() : dumpModuleSymsForObj();
    if (EC)
      return joinErrors(createStringError(inconvertibleErrorCode(), "Unexpected error processing modules:"), EC);
  }
```


================
Comment at: llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp:682
+
+            if (SG.getFile().isPdb()) {
+              AutoIndent Indent(P);
----------------
You can early exit here.


================
Comment at: llvm/tools/llvm-pdbutil/InputFile.cpp:40
 getModuleDebugStream(PDBFile &File, StringRef &ModuleName, uint32_t Index) {
-  ExitOnError Err("Unexpected error: ");
-
-  auto &Dbi = Err(File.getPDBDbiStream());
+  auto DbiOrErr = File.getPDBDbiStream();
+  if (!DbiOrErr)
----------------
Even though `auto` is already used throughout this file, I find it's hard for a person reading the code to understand what type the function returns. Also please see https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable

It's probably better if we wrote `Expected<DbiStream &> DbiOrErr = ...` and `DbiStream &Dbi = ...` below. Same applies for other places/usage of `auto` throughout this patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121801/new/

https://reviews.llvm.org/D121801