[PATCH] D62276: lld-link, clang: Treat non-existent input files as possible spellos for option flags

[Nico Weber via Phabricator via cfe-commits]

thakis marked 5 inline comments as done.
thakis added a comment.

Thanks! Landing with comments addressed.



================
Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:177
 def warn_drv_unknown_argument_clang_cl_with_suggestion : Warning<
-  "unknown argument ignored in clang-cl '%0' (did you mean '%1'?)">,
+  "unknown argument ignored in clang-cl '%0', did you mean '%1'?">,
   InGroup<UnknownArgument>;
----------------
hans wrote:
> (grammar nit: I think this is a comma splice (https://en.wikipedia.org/wiki/Comma_splice)  so I believe a semicolon would be better. On the other hand, we seem to do this a lot already and I'm not a native speaker so maybe it's fine.)
Looking through `rg 'did you mean' clang/include/clang/Basic/` we're actually pretty consistent about using a semicolon everywhere except for this file, so changed to ;. Thanks!


================
Comment at: clang/include/clang/Driver/Driver.h:398
+  /// Check that the file referenced by Value exists. If it doesn't,
+  /// issue a diagnostic and return false.
+  bool DiagnoseInputExistence(const llvm::opt::DerivedArgList &Args,
----------------
hans wrote:
> Should the comment say what TypoCorrect does?
Sure, added.


================
Comment at: lld/COFF/Driver.cpp:218
+      else
+        error(Error + "; did you mean '" + Nearest + "'");
+    } else
----------------
hans wrote:
> I like the semicolon better, but it seems we use comma in other places?
This is a semicolon because lld-link includes the strerror output (`.second.message()`), so this looks like `could not open 'foo': No such file; did you mean '/foo'`. Without the semicolon it looks like the "did you mean" is part of the error message.


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

https://reviews.llvm.org/D62276