[Option] Fix PR37006 prefix choice in findNearest

Summary:
In https://bugs.llvm.org/show_bug.cgi?id=37006 Nico Weber points out a
flaw in `OptTable::findNearest`: if an option "foo"'s prefixes are "--"
and "-", then the nearest option for "--fob" will be "-foo". This is
incorrect, however, since the function is expected to return "--foo".

The bug is due to a naive loop that attempts to predetermines which
prefix is best. Instead, compute the edit distance for each prefix/name
pair.

Test Plan: `check-llvm`

Reviewers: thakis

Reviewed By: thakis

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D46776

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@332299 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Option/OptTable.cpp b/lib/Option/OptTable.cpp
index 022b9d5..293b6e5 100644 (file)
--- a/lib/Option/OptTable.cpp
+++ b/lib/Option/OptTable.cpp
@@ -252,38 +252,33 @@ unsigned OptTable::findNearest(StringRef Option, std::string &NearestString,
                                unsigned MinimumLength) const {
   assert(!Option.empty());
 
-  // Consider each option as a candidate, finding the closest match.
+  // Consider each [option prefix + option name] pair as a candidate, finding
+  // the closest match.
   unsigned BestDistance = UINT_MAX;
   for (const Info &CandidateInfo :
        ArrayRef<Info>(OptionInfos).drop_front(FirstSearchableIndex)) {
     StringRef CandidateName = CandidateInfo.Name;
 
-    // Ignore option candidates with empty names, such as "--", or names
-    // that do not meet the minimum length.
+    // We can eliminate some option prefix/name pairs as candidates right away:
+    // * Ignore option candidates with empty names, such as "--", or names
+    //   that do not meet the minimum length.
     if (CandidateName.empty() || CandidateName.size() < MinimumLength)
       continue;
 
-    // If FlagsToInclude were specified, ignore options that don't include
-    // those flags.
+    // * If FlagsToInclude were specified, ignore options that don't include
+    //   those flags.
     if (FlagsToInclude && !(CandidateInfo.Flags & FlagsToInclude))
       continue;
-    // Ignore options that contain the FlagsToExclude.
+    // * Ignore options that contain the FlagsToExclude.
     if (CandidateInfo.Flags & FlagsToExclude)
       continue;
 
-    // Ignore positional argument option candidates (which do not
-    // have prefixes).
+    // * Ignore positional argument option candidates (which do not
+    //   have prefixes).
     if (!CandidateInfo.Prefixes)
       continue;
-    // Find the most appropriate prefix. For example, if a user asks for
-    // "--helm", suggest "--help" over "-help".
-    StringRef Prefix = CandidateInfo.Prefixes[0];
-    for (int P = 1; CandidateInfo.Prefixes[P]; P++) {
-      if (Option.startswith(CandidateInfo.Prefixes[P]))
-        Prefix = CandidateInfo.Prefixes[P];
-    }
 
-    // Check if the candidate ends with a character commonly used when
+    // Now check if the candidate ends with a character commonly used when
     // delimiting an option from its value, such as '=' or ':'. If it does,
     // attempt to split the given option based on that delimiter.
     std::string Delimiter = "";
@@ -297,14 +292,19 @@ unsigned OptTable::findNearest(StringRef Option, std::string &NearestString,
     else
       std::tie(LHS, RHS) = Option.split(Last);
 
-    std::string NormalizedName =
-        (LHS.drop_front(Prefix.size()) + Delimiter).str();
-    unsigned Distance =
-        CandidateName.edit_distance(NormalizedName, /*AllowReplacements=*/true,
-                                    /*MaxEditDistance=*/BestDistance);
-    if (Distance < BestDistance) {
-      BestDistance = Distance;
-      NearestString = (Prefix + CandidateName + RHS).str();
+    // Consider each possible prefix for each candidate to find the most
+    // appropriate one. For example, if a user asks for "--helm", suggest
+    // "--help" over "-help".
+    for (int P = 0; const char *const CandidatePrefix = CandidateInfo.Prefixes[P]; P++) {
+      std::string NormalizedName = (LHS + Delimiter).str();
+      StringRef Candidate = (CandidatePrefix + CandidateName).str();
+      unsigned Distance =
+          Candidate.edit_distance(NormalizedName, /*AllowReplacements=*/true,
+                                  /*MaxEditDistance=*/BestDistance);
+      if (Distance < BestDistance) {
+        BestDistance = Distance;
+        NearestString = (Candidate + RHS).str();
+      }
     }
   }
   return BestDistance;

diff --git a/unittests/Option/OptionParsingTest.cpp b/unittests/Option/OptionParsingTest.cpp
index eef21ab..99c1ef3 100644 (file)
--- a/unittests/Option/OptionParsingTest.cpp
+++ b/unittests/Option/OptionParsingTest.cpp
@@ -283,6 +283,10 @@ TEST(Option, FindNearest) {
   EXPECT_EQ(Nearest, "-blorp");
   EXPECT_EQ(1U, T.findNearest("--blorm", Nearest));
   EXPECT_EQ(Nearest, "--blorp");
+  EXPECT_EQ(1U, T.findNearest("-blarg", Nearest));
+  EXPECT_EQ(Nearest, "-blarn");
+  EXPECT_EQ(1U, T.findNearest("--blarm", Nearest));
+  EXPECT_EQ(Nearest, "--blarn");
   EXPECT_EQ(1U, T.findNearest("-fjormp", Nearest));
   EXPECT_EQ(Nearest, "--fjormp");
 

diff --git a/unittests/Option/Opts.td b/unittests/Option/Opts.td
index c4544b5..e6151d3 100644 (file)
--- a/unittests/Option/Opts.td
+++ b/unittests/Option/Opts.td
@@ -30,6 +30,7 @@ def Slurp : Option<["-"], "slurp", KIND_REMAINING_ARGS>;
 def SlurpJoined : Option<["-"], "slurpjoined", KIND_REMAINING_ARGS_JOINED>;
 
 def Blorp : Flag<["-", "--"], "blorp">, HelpText<"The blorp option">, Flags<[OptFlag1]>;
+def Blarn : Flag<["--", "-"], "blarn">, HelpText<"The blarn option">, Flags<[OptFlag1]>;
 def Cramb : Joined<["/"], "cramb:">, HelpText<"The cramb option">, MetaVarName<"CRAMB">, Flags<[OptFlag1]>;
 def Doopf1 : Flag<["-"], "doopf1">, HelpText<"The doopf1 option">, Flags<[OptFlag1]>;
 def Doopf2 : Flag<["-"], "doopf2">, HelpText<"The doopf2 option">, Flags<[OptFlag2]>;