From 07541493d51a303a90ad6c63c173b6545f8d604f Mon Sep 17 00:00:00 2001 From: Brian Gesiak Date: Sat, 19 May 2018 12:03:26 +0000 Subject: [PATCH] Un-revert "[Option] Fix PR37006 prefix choice in findNearest" Summary: In https://reviews.llvm.org/rL332804 I loosed the assertion in the Clang driver test that forced me to revert https://reviews.llvm.org/rL332299. Once this lands I should be able to narrow down what caused PS4 buildbots to fail, and reinstate the check in that test. Test Plan: check-llvm & check-clang git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@332805 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 022b9d5d933..293b6e54e21 100644 --- 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(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 eef21ab5120..99c1ef32476 100644 --- 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 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]>;