aganea accepted this revision. aganea added a comment. LGTM.
================ 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); ---------------- mstorsjo wrote: > 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. I see, thanks for the explanation! 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