[llvm] r359580 - Re-reland "[Option] Fix PR37006 prefix choice in findNearest"
Nico Weber via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 30 10:46:00 PDT 2019
Author: nico
Date: Tue Apr 30 10:46:00 2019
New Revision: 359580
URL: http://llvm.org/viewvc/llvm-project?rev=359580&view=rev
Log:
Re-reland "[Option] Fix PR37006 prefix choice in findNearest"
This was first reviewed in https://reviews.llvm.org/D46776 and
landed in r332299, but got reverted because it broke the PS4
bots.
https://reviews.llvm.org/D50410 fixed this, and then this
change was re-reviewed at https://reviews.llvm.org/D50515 and
relanded in r341329. It got reverted due to causing MSan issues.
However, nobody wrote down the error message and the bot link
is dead, so I'm relanding this to capture the MSan error.
I'll then either fix it, or copy it somewhere and revert if
fixing looks difficult.
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=359580&r1=359579&r2=359580&view=diff
==============================================================================
--- llvm/trunk/lib/Option/OptTable.cpp (original)
+++ llvm/trunk/lib/Option/OptTable.cpp Tue Apr 30 10:46:00 2019
@@ -251,38 +251,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 = "";
@@ -296,14 +291,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=359580&r1=359579&r2=359580&view=diff
==============================================================================
--- llvm/trunk/unittests/Option/OptionParsingTest.cpp (original)
+++ llvm/trunk/unittests/Option/OptionParsingTest.cpp Tue Apr 30 10:46:00 2019
@@ -286,6 +286,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=359580&r1=359579&r2=359580&view=diff
==============================================================================
--- llvm/trunk/unittests/Option/Opts.td (original)
+++ llvm/trunk/unittests/Option/Opts.td Tue Apr 30 10:46:00 2019
@@ -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