[PATCH] D70769: [Support] add vfs support for ExpandResponseFiles

[liu hui via Phabricator via cfe-commits]

lh123 marked 3 inline comments as done.
lh123 added inline comments.


================
Comment at: llvm/lib/Support/CommandLine.cpp:1069
+      return llvm::createStringError(
+          std::make_error_code(std::errc::illegal_byte_sequence),
+          "Could not convert UTF16 To UTF8");
----------------
jhenderson wrote:
> lh123 wrote:
> > jhenderson wrote:
> > > jhenderson wrote:
> > > > `std::make_error_code(std::errc::illegal_byte_sequence)` -> `errc::illegal_byte_sequence`
> > > LLVM has its own errc error_code set. Please use that, i.e. delete the `std::` (note that I didn't add `std::` in my previous comment).
> > I think we should use `std::errc::illegal_byte_sequence`, because it's sigature is:
> >  ```
> > template <typename... Ts>
> > inline Error createStringError(std::errc EC, char const *Fmt, const Ts &... Vals)
> > ```
> I'm not sure if that's a mistake in the function interface or not, and it looks like our usage is very inconsistent, but the comments in Errc.h imply that we really should use llvm::errc values and not std::errc, or there may be problems.
I think `llvm::errc` and `std::errc` are both correct, `llvm::errc` eventually implicitly converts to `std::error_code`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70769