[patch] Use the DiagnosticHandler to print diagnostics when reading bitcode.

[Justin Bogner]

Rafael Espíndola <rafael.espindola at gmail.com> writes:
> Currently the bitcode reading interface uses std::error_code to report
> an error to the callers and it is the callers job to print
> diagnostics.
>
> This is not ideal for error handling or diagnostic reporting:
>
> * For error handling, all that the callers care about is 3 possibilities:
>   * It worked
>   * The bitcode file is corrupted/invalid.
>   * The file is not bitcode at all.
>
> * For diagnostic, it is user friendly to include far more information
> about the invalid case so the user can find out what is wrong with the
> bitcode file. This comes up for example when a developer introduces a
> bug while extending the format.
>
> The compromise we have currently is to have a lot of error codes.
>
> With this patch we use the DiagnosticHandler to communicate with the
> human and std::error_code to communicate with the caller.
>
> This allows us to have far fewer error codes and adds the
> infrastructure to print better diagnostics. This is so because the
> diagnostics are printed when the issue is found. The code that
> detected the problem in alive in the stack and can pass down as much
> context as needed. As an example the patch updates
> test/Bitcode/invalid.ll.
>
> Using a DiagnosticHandler also moves the fatal/non-fatal error
> decision to the caller. A simple one like llvm-dis can just use fatal
> errors. The gold plugin needs a bit more complex treatment because of
> being passed non-bitcode files. An hypothetical interactive tool would
> make all bitcode errors non-fatal.
 ...
> +static void diagnosticHandler(const DiagnosticInfo &DI, void *Context) {
> +  assert(DI.getSeverity() == DS_Error && "Only expecting errors");
> +
> +  raw_ostream &OS = errs();
> +  OS << (char *)Context << ": ";
> +  DiagnosticPrinterRawOStream DP(OS);
> +  DI.print(DP);
> +  OS << '\n';
> +  exit(1);
> +}
> +
>  int main(int argc, char **argv) {
>    // Print a stack trace if we signal out.
>    sys::PrintStackTraceOnErrorSignal();
> @@ -120,6 +133,7 @@ int main(int argc, char **argv) {
>    LLVMContext &Context = getGlobalContext();
>    llvm_shutdown_obj Y;  // Call llvm_shutdown() on exit.
>  
> +  Context.setDiagnosticHandler(diagnosticHandler, argv[0]);
>  
>    cl::ParseCommandLineOptions(argc, argv, "llvm .bc -> .ll disassembler\n");
>  
> @@ -136,25 +150,8 @@ int main(int argc, char **argv) {
>        DisplayFilename = InputFilename;
>      ErrorOr<std::unique_ptr<Module>> MOrErr =
>          getStreamedBitcodeModule(DisplayFilename, Streamer, Context);
> -    if (std::error_code EC = MOrErr.getError())
> -      ErrorMessage = EC.message();
> -    else
> -      M = std::move(*MOrErr);
> -    if(M.get()) {
> -      if (std::error_code EC = M->materializeAllPermanently()) {
> -        ErrorMessage = EC.message();
> -        M.reset();
> -      }
> -    }
> -  }
> -
> -  if (!M.get()) {
> -    errs() << argv[0] << ": ";
> -    if (ErrorMessage.size())
> -      errs() << ErrorMessage << "\n";
> -    else
> -      errs() << "bitcode didn't read correctly.\n";
> -    return 1;
> +    M = std::move(*MOrErr);

It's probably better to leave the `if (hasError()) return 1` path in
here, just to future proof against changes to the diagnosticHandler.

> +    M->materializeAllPermanently();
>    }
>  
>    // Just use stdout.  We won't actually print anything on it.
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits