[PATCH] D136090: Handle errors in expansion of response files

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.


================
Comment at: clang/lib/Driver/Driver.cpp:1029
         if (llvm::sys::path::is_relative(CfgFilePath)) {
-          if (getVFS().makeAbsolute(CfgFilePath))
-            return true;
-          auto Status = getVFS().status(CfgFilePath);
-          if (!Status ||
-              Status->getType() != llvm::sys::fs::file_type::regular_file) {
-            Diag(diag::err_drv_config_file_not_exist) << CfgFilePath;
+          if (std::error_code EC = getVFS().makeAbsolute(CfgFilePath)) {
+            Diag(diag::err_drv_cannot_open_config_file)
----------------
mgorny wrote:
> Either I'm missing something or you're not using `EC`.
You are right, this is remnants of previous implementation. Thanks for the catch.


================
Comment at: clang/lib/Driver/Driver.cpp:1041
+        }
+        if (Status->getType() != llvm::sys::fs::file_type::regular_file) {
+          Diag(diag::err_drv_cannot_open_config_file)
----------------
mgorny wrote:
> Not necessarily a goal for this patch but wouldn't this check fit better in `readConfigFile()`?
Yes, it makes sense. Moved.


================
Comment at: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp:67-69
+      if (Err) {
+        llvm::errs() << Err;
+      }
----------------
mgorny wrote:
> The braces seem unnecessary.
Fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136090