jhenderson added inline comments.
================
Comment at: llvm/lib/Support/CommandLine.cpp:1148
+ llvm::ErrorOr<llvm::vfs::Status> RHS = FS.status(RFile.File);
+ if (!LHS || !RHS) {
+ return false;
----------------
kadircet wrote:
> again you need to `consumeError`s before returning. I would first get `LHS`,
> consume and bail out if it was an error, then do the same for `RHS` and only
> after that return `equivalent`.
Could you add a TODO comment here and at the other `consumeError()` call to say
that the error should be propagated up the stack, please?
================
Comment at: llvm/lib/Support/CommandLine.cpp:1054
+ if (!CurrDirOrErr) {
+ llvm::consumeError(llvm::errorCodeToError(CurrDirOrErr.getError()));
return false;
----------------
I'd ultimately like to see the public interface return
llvm::Expected/llvm::Error so that a potentially useful error with information
isn't just thrown away. However, I agree that that's probably a separate
change. On the other hand, it should be simple to return it here, since this is
only used locally, for consumption in `ExpandResponseFiles` below. Could you
make that change, please?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70769/new/
https://reviews.llvm.org/D70769
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
