[llvm] r332299 - [Option] Fix PR37006 prefix choice in findNearest
Brian Gesiak via llvm-commits
llvm-commits at lists.llvm.org
Mon May 14 14:35:00 PDT 2018
Author: modocache
Date: Mon May 14 14:35:00 2018
New Revision: 332299
URL: http://llvm.org/viewvc/llvm-project?rev=332299&view=rev
Log:
[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
Modified:
llvm/trunk/lib/Option/OptTable.cpp
llvm/trunk/unittests/Option/OptionParsingTest.cpp
llvm/trunk/unittests/Option/Opts.td
Modified: llvm/trunk/lib/Option/OptTable.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Option/OptTable.cpp?rev=332299&r1=332298&r2=332299&view=diff
==============================================================================
--- llvm/trunk/lib/Option/OptTable.cpp (original)
+++ llvm/trunk/lib/Option/OptTable.cpp Mon May 14 14:35:00 2018
@@ -252,38 +252,33 @@ unsigned OptTable::findNearest(StringRef
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
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;
Modified: llvm/trunk/unittests/Option/OptionParsingTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Option/OptionParsingTest.cpp?rev=332299&r1=332298&r2=332299&view=diff
==============================================================================
--- llvm/trunk/unittests/Option/OptionParsingTest.cpp (original)
+++ llvm/trunk/unittests/Option/OptionParsingTest.cpp Mon May 14 14:35:00 2018
@@ -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");
Modified: llvm/trunk/unittests/Option/Opts.td
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Option/Opts.td?rev=332299&r1=332298&r2=332299&view=diff
==============================================================================
--- llvm/trunk/unittests/Option/Opts.td (original)
+++ llvm/trunk/unittests/Option/Opts.td Mon May 14 14:35:00 2018
@@ -30,6 +30,7 @@ def Slurp : Option<["-"], "slurp", KIND_
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]>;
More information about the llvm-commits
mailing list