[PATCH] D49736: [Basic] Emit warning flag suggestion only in case there's *similar* existing flag for given unknown one

[Jan Korous via Phabricator via cfe-commits]

jkorous created this revision.
jkorous added reviewers: arphaman, vsapsai, bruno, JDevlieghere.
Herald added subscribers: cfe-commits, dexonsmith.

In case user specified non-existent warning flag clang was suggesting an 
existing on that was in some cased very different. This patch should ensure 
that either a relevant warning flag is suggested or none (which is in line with 
unknown generic command line option handling).

  Examples:
  warning: unknown warning option '-Whiskey'; did you mean '-Wpacked'?
  warning: unknown warning option '-Weverthin'; did you mean '-Wsection'?
  
  rdar://problem/37755482

I changed interface of DiagnosticIDs::getNearestOption() to return the edit 
distance between nonexistent flag and the closest exiting one. In principle 
this code is analogous to OptTable::findNearest() for generic command line 
options but I don't find them similar enough in order to factor out some shared 
implementation. I tentatively selected maximum edit distance as 2 
(CompilerInvocation::CreateFromArgs() uses maximum edit distance of 1 for 
generic case).


Repository:
  rC Clang

https://reviews.llvm.org/D49736

Files:
  Basic/DiagnosticIDs.cpp
  Basic/Warnings.cpp
  Driver/cc-log-diagnostics.c
  Frontend/warning-options.cpp
  clang/Basic/DiagnosticIDs.h


Index: Frontend/warning-options.cpp
===================================================================
--- Frontend/warning-options.cpp
+++ Frontend/warning-options.cpp
@@ -3,5 +3,5 @@
 // CHECK: unknown warning option '-Wmonkey'
 // CHECK: unknown warning option '-Wno-monkey'
 // CHECK: unknown warning option '-Wno-unused-command-line-arguments'; did you mean '-Wno-unused-command-line-argument'?
-// CHECK: unknown warning option '-Wmodule-build'; did you mean '-Wmodule-conflict'?
+// CHECK: unknown warning option '-Wmodule-build'
 // CHECK: unknown remark option '-Rmodule-built'; did you mean '-Rmodule-build'?
Index: Driver/cc-log-diagnostics.c
===================================================================
--- Driver/cc-log-diagnostics.c
+++ Driver/cc-log-diagnostics.c
@@ -17,7 +17,7 @@
 // CHECK:       <key>level</key>
 // CHECK:       <string>warning</string>
 // CHECK:       <key>message</key>
-// CHECK:       <string>unknown warning option &apos;-Wfoobar&apos;; did you mean &apos;-W{{.*}}&apos;?</string>
+// CHECK:       <string>unknown warning option &apos;-Wfoobar&apos;</string>
 // CHECK:     </dict>
 // CHECK:     <dict>
 // CHECK:       <key>level</key>
Index: Basic/Warnings.cpp
===================================================================
--- Basic/Warnings.cpp
+++ Basic/Warnings.cpp
@@ -30,15 +30,24 @@
 #include <utility>
 using namespace clang;
 
+namespace {
+  constexpr unsigned MaxLevenshteinDistForWarningSuggestion = 2;
+}
+
 // EmitUnknownDiagWarning - Emit a warning and typo hint for unknown warning
 // opts
 static void EmitUnknownDiagWarning(DiagnosticsEngine &Diags,
                                    diag::Flavor Flavor, StringRef Prefix,
                                    StringRef Opt) {
-  StringRef Suggestion = DiagnosticIDs::getNearestOption(Flavor, Opt);
+  std::string Suggestion;
+  const unsigned OptToSuggestionEditDist
+    = DiagnosticIDs::getNearestOption(Flavor, Opt, Suggestion);
   Diags.Report(diag::warn_unknown_diag_option)
     << (Flavor == diag::Flavor::WarningOrError ? 0 : 1) << (Prefix.str() += Opt)
-    << !Suggestion.empty() << (Prefix.str() += Suggestion);
+    << (!Suggestion.empty()
+      && OptToSuggestionEditDist > 0
+      && OptToSuggestionEditDist <= MaxLevenshteinDistForWarningSuggestion)
+    << (Prefix.str() += Suggestion);
 }
 
 void clang::ProcessWarningOptions(DiagnosticsEngine &Diags,
Index: Basic/DiagnosticIDs.cpp
===================================================================
--- Basic/DiagnosticIDs.cpp
+++ Basic/DiagnosticIDs.cpp
@@ -589,9 +589,9 @@
       Diags.push_back(StaticDiagInfo[i].DiagID);
 }
 
-StringRef DiagnosticIDs::getNearestOption(diag::Flavor Flavor,
-                                          StringRef Group) {
-  StringRef Best;
+unsigned DiagnosticIDs::getNearestOption(diag::Flavor Flavor,
+                                          StringRef Group,
+                                          std::string& NearestOption) {
   unsigned BestDistance = Group.size() + 1; // Sanity threshold.
   for (const WarningOption &O : OptionTable) {
     // Don't suggest ignored warning flags.
@@ -609,15 +609,15 @@
 
     if (Distance == BestDistance) {
       // Two matches with the same distance, don't prefer one over the other.
-      Best = "";
+      NearestOption = "";
     } else if (Distance < BestDistance) {
       // This is a better match.
-      Best = O.getName();
+      NearestOption = O.getName();
       BestDistance = Distance;
     }
   }
 
-  return Best;
+  return BestDistance;
 }
 
 /// ProcessDiag - This is the method used to report a diagnostic that is
Index: clang/Basic/DiagnosticIDs.h
===================================================================
--- clang/Basic/DiagnosticIDs.h
+++ clang/Basic/DiagnosticIDs.h
@@ -300,8 +300,12 @@
                                 std::vector<diag::kind> &Diags);
 
   /// Get the diagnostic option with the closest edit distance to the
-  /// given group name.
-  static StringRef getNearestOption(diag::Flavor Flavor, StringRef Group);
+  /// given Group name and return their edit distance.
+  /// \param [out] NearestOption - The nearest option string found.
+  ///
+  /// \return The edit distance between Group and the nearest option found.
+  static unsigned getNearestOption(diag::Flavor Flavor, StringRef Group,
+                                   std::string& NearestOption);
 
 private:
   /// Classify the specified diagnostic ID into a Level, consumable by

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits