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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
