Patch: "Did you mean" warning correction handles negative options poorly

The “did you mean” for warnings first removes the ‘-Wno-’, if present, and then only matches against positive options. This leads to some annoying problems, like:

$ clang++ -Wnoarc
clang: warning: unknown warning option ‘-Wnoarc’; did you mean ‘-Warc’?

When what I really want is a suggestion to use ‘-Wno-arc’

Attached is a patch that takes a stab at dealing with this. It does in principle make the existing code slower, my turning a bunch of StringRefs into std::strings, but I don’t imagine spell-checking warning options is on the critical path!

I tried adding a test, which is purposefully broken (note the ’ PURPOSEFUL BREAK ’ comment in it), but which doesn’t seem to be getting run (and failing). I looked around and could not figure out why it is not getting invoked, any comments welcome.


warning-no.patch (4.66 KB)

The test being broken probably has to do with the fact that the file
extension is ".cc"; we generally prefer ".cpp" in LLVM.

@@ -36,11 +36,11 @@
static void EmitUnknownDiagWarning(DiagnosticsEngine &Diags,
                                   StringRef Prefix, StringRef Opt,
                                   bool isPositive) {
- StringRef Suggestion = DiagnosticIDs::getNearestWarningOption(Opt);
+ std::string Suggestion =
DiagnosticIDs::getNearestWarningOption(Prefix.str() += Opt);

Not your fault, but "Prefix.str() += Opt" is a really weird thing to
write. I don't think we care about performance here, so "Prefix.str()
+ Opt.str()" would be fine. (If performance could matter, llvm::Twine
and llvm::SmallString are useful.)

I'm not sure passing getNearestWarningOption the full warning flag is
the right approach here: I haven't tested, but I would guess this
patch breaks typo correction for e.g. "-Werror=arc".