[PATCH] D46776: [Option] Fix PR37006 prefix choice in findNearest

[Brian Gesiak via Phabricator via llvm-commits]

modocache added inline comments.


================
Comment at: lib/Option/OptTable.cpp:284
-        Prefix = CandidateInfo.Prefixes[P];
-    }
 
----------------
thakis wrote:
> Shouldn't getting the longest prefix here be good enough? Something like this:
> 
> ```
> StringRef Prefix;
> int PrefixLength = 0;
> for (int P = 0; Candidateinfo.Prefixes[P]; P++) {
>   StringRef Candidate = CandidateInfo.Prefixes[P];
>   if (Candiate.size() > PrefixLength && Option.startswith(Candidate)) {
>     Prefix = Candidate;
>     PrefixLength = Candidate.size();
>   }
> }
> ```
> 
> …and keep everything else the same?
That's a good suggestion, but here's a case in which this loop wouldn't work: the user invokes `clang -migrate foo.cpp`, but the correct argument is `--migrate`. In this case, the `--migrate` option does not provide a `-` prefix, it only provides `--`. However the `--` prefix is ruled out by the `if` statement above: `-migrate` does not start with `--`.

(The unit tests managed to catch the edge case above, hooray!)

My personal preference would be to check the edit distance of each prefix/name pair, as this diff currently does. It's slightly more computationally expensive, but I think it's a more comprehensive check.


Repository:
  rL LLVM

https://reviews.llvm.org/D46776