[PATCH] D39203: [raw_fd_ostream] report actual error in error messages

[Reid Kleckner via Phabricator via llvm-commits]

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

In https://reviews.llvm.org/D39203#904129, @inglorion wrote:

> As an alternative, I considered using an Error instead of an error_code. I decided against that, because
>
> - It produces a riskier change. Error.h depends on raw_ostream.h, and now we would also have an edge going the other way. Working around this is possible, but can affect clients that depend on Error.h, of which there are many.


Separately, we should cut this edge if possible. Widely included headers like Error.h should have as few transitive includes as possible.

> - It changes the semantics from unchecked errors resulting in fatals to unchecked errors *or successes* resulting in fatals. This could be worked around, but it would make the change larger comparing to just using a type that already provides compatible semantics.
> - raw_ostream et al. are meant to look like standard C++ types. std::error_code is arguably a better fit for that than llvm::Error.
> - The code that raw_fd_ostream calls already produces error_codes, so by just using those, we avoid having to convert them to Error.
> 
>   In short, std::error_code makes for a simpler, less risky change that fits with the existing code and semantics, so I chose to go with that.

I totally agree. :)

Looks good!



================
Comment at: llvm/lib/LTO/LTOCodeGenerator.cpp:263
   if (objFile.os().has_error()) {
-    emitError((Twine("could not write object file: ") + Filename).str());
+    emitError((Twine("could not write object file: ") + Filename + ": " +
+               objFile.os().error().message())
----------------
Separately, emitError and emitWarning should really take something other than std::string.


https://reviews.llvm.org/D39203