Bigcheese wrote: The other cases of `std::system_category` were in `LLVM_ON_UNIX` (or similar) blocks that would only be used on systems where it's mostly fine to use either one, as most of the time you'll get an error that's in `std::errc`, and then there's no difference (or they just are never compared in general). The initial desire to do this came from spending 30m looking into which one to use on UNIX systems in general and wanting to avoid that in the future. The `JSONTransport.cpp` case was just more indication to me that the existing way was error prone.
In auditing all uses of `errno` I did find a few other places where the code isn't quite wrong, but it's not really using `llvm::Error` correctly. There's quite a few places where people use `llvm::inconvertibleErrorCode()` where they really shouldn't be. For example `llvm-exegesis` has a bunch of places where they call `strerror(errno)` to construct an `llvm::Error` that implicitly uses `llvm::inconvertibleErrorCode()` as the `std::error_code` value. Our existing documentation here is pretty nice for `Error` and `Expected` (https://llvm.org/docs/ProgrammersManual.html#error-handling), but it would be nice to better cover how `std::error_code` is supposed to be propagated, as it currently kind of implies they are going away, but they are always needed for OS errors. I'll try to get to this when I can find time. As for test coverage, some of these have existing error tests, but for a lot of the rest it's either incredibly difficult or just not possible (without using `LD_PRELOAD` or something similar) to test. Given that, it's good to make handling them as easy to get correct as practicable. https://github.com/llvm/llvm-project/pull/84423 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
