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

[Kadir Cetinkaya via Phabricator via llvm-commits]

kadircet added inline comments.


================
Comment at: llvm/lib/Support/CommandLine.cpp:1053
+  llvm::ErrorOr<std::string> CurrDir = FS.getCurrentWorkingDirectory();
+  if (!CurrDir)
+    return false;
----------------
this comment has moved into a irrelevant line and wasn't addressed, so re-stating it here:

```
it is sad that, ExpandResponseFile returns a bool, but that's a battle for another day.

unfortunately, it is not enough return false in this case you need to consume the error with llvm::consumeError before exiting the scope.
```


================
Comment at: llvm/lib/Support/CommandLine.cpp:1148
+      llvm::ErrorOr<llvm::vfs::Status> RHS = FS.status(RFile.File);
+      if (!LHS || !RHS) {
+        return false;
----------------
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`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70769