This is an archive of the discontinued LLVM Phabricator instance.

lld-link, clang: Treat non-existent input files as possible spellos for option flags
ClosedPublic

Authored by thakis on May 22 2019, 5:28 PM.

Details

Summary

OptTable treats arguments starting with / that aren't a known option
as filenames. This means lld-link's and clang-cl's typo correction for
unknown flags didn't do spell checking for misspelled options that start
with /.

I first tried changing OptTable, but that got pretty messy, see PR41787
comments 2 and 3.

Instead, let lld-link's and clang's (including clang-cl's) "file not
found" diagnostic check if a non-existent file looks like it could be a
mis-spelled option, and if so add a "did you mean" suggestion to the
"file not found" diagnostic.

While here, make formatting of a few diagnostics a bit more
self-consistent.

Fixes PR41787.

Diff Detail

Event Timeline

thakis created this revision.May 22 2019, 5:28 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 22 2019, 5:28 PM
hans accepted this revision.May 23 2019, 12:26 AM

Beautiful!

Want to add a line to docs/ReleaseNotes.rst too? :-)

clang/include/clang/Basic/DiagnosticDriverKinds.td
177

(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.)

clang/include/clang/Driver/Driver.h
398

Should the comment say what TypoCorrect does?

clang/lib/Driver/Driver.cpp
2024

indentation looks funny; clang-format?

lld/COFF/Driver.cpp
218

I like the semicolon better, but it seems we use comma in other places?

This revision is now accepted and ready to land.May 23 2019, 12:26 AM
thakis marked 5 inline comments as done.May 23 2019, 10:39 AM

Thanks! Landing with comments addressed.

clang/include/clang/Basic/DiagnosticDriverKinds.td
177

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!

clang/include/clang/Driver/Driver.h
398

Sure, added.

lld/COFF/Driver.cpp
218

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.

rnk accepted this revision.May 23 2019, 10:42 AM

Thanks, this has been a longstanding issue!

This revision was automatically updated to reflect the committed changes.