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

[Carlos Alberto Enciso via Phabricator via llvm-commits]

CarlosAlbertoEnciso added inline comments.


================
Comment at: llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp:455
 
-  ExitOnError Err("Unexpected error processing modules: ");
-
----------------
aganea wrote:
> 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);
>   }
> ```
Very valid point.

The following functions are affected:

```
Error DumpOutputStyle::dumpModules();
Error DumpOutputStyle::dumpModuleFiles();
Error DumpOutputStyle::dumpSymbolStats();
Error DumpOutputStyle::dumpModuleSymsForPdb();

```
What I am suggesting is to pass an additional parameter to the functions that are intended to be moved to the PDB library that now are returning an Error and add your suggested code to those functions.

```
if (EC)
    return joinErrors(createStringError(inconvertibleErrorCode(), ExtraMessage), EC);
```



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


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

https://reviews.llvm.org/D121801