From ed4e4a5f8d362175f645fc7ec6e48b90d9ad38b1 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 30 Apr 2019 17:46:00 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@359580 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Option/OptTable.cpp | 48 +++++++++++++------------- unittests/Option/OptionParsingTest.cpp | 4 +++ unittests/Option/Opts.td | 1 + 3 files changed, 29 insertions(+), 24 deletions(-) diff --git a/lib/Option/OptTable.cpp b/lib/Option/OptTable.cpp index c0a1861a8d5..8956c6830db 100644 --- a/lib/Option/OptTable.cpp +++ b/lib/Option/OptTable.cpp @@ -251,38 +251,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(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 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 ae3f6522eda..f6b6b95a209 100644 --- a/unittests/Option/OptionParsingTest.cpp +++ b/unittests/Option/OptionParsingTest.cpp @@ -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"); diff --git a/unittests/Option/Opts.td b/unittests/Option/Opts.td index c4544b5b3f9..e6151d375ef 100644 --- 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]>; -- 2.40.0