mstorsjo added inline comments.

================
Comment at: llvm/tools/llvm-rc/llvm-rc.cpp:94
+
+ErrorOr<std::string> findClang(const char *Argv0) {
+  StringRef Parent = llvm::sys::path::parent_path(Argv0);
----------------
aganea wrote:
> Since you're not using the `std::error_code` below in the call site, should 
> this return `Expected<...>`? In that case, the variable `Path` shouldn't be 
> needed?
With `Expected<>` I'd need to craft an `Error` at the end if I don't have a 
path to return, but do you mean `Optional<>`? I guess that'd work - but as 
`findProgramByName()` returns `ErrorOr<std::string>` I kept the same signature.

Even if returning `Optional<>`, we need a local variable to receive the 
`ErrorOr<std::string>` from `findProgramByName()` and inspect it.

In the future (after a release or so) I'd intend for this to be a hard error, 
so at that point, the returned error code actually would be printed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100755

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to