[cfe-dev] Patch: "Did you mean" warning correction handles negative options poorly

Eli Friedman eli.friedman at gmail.com
Fri Nov 9 15:15:06 PST 2012

On Thu, Nov 8, 2012 at 4:33 AM, Chris Jefferson <chris at bubblescope.net> wrote:
> 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.

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".


More information about the cfe-dev mailing list