[PATCH] D126159: [ADT] Add edit_distance_insensitive to StringRef

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/include/llvm/ADT/edit_distance.h:45
 /// the given sequences into the other. If zero, the sequences are identical.
-template<typename T>
-unsigned ComputeEditDistance(ArrayRef<T> FromArray, ArrayRef<T> ToArray,
-                             bool AllowReplacements = true,
-                             unsigned MaxEditDistance = 0) {
+template <typename T, typename Functor = const T &(*)(const T &)>
+unsigned ComputeEditDistance(
----------------
Do you need the default type argument here? The default (or explicit, in the other call) value below would allow template argument deduction, right?


================
Comment at: llvm/include/llvm/ADT/edit_distance.h:49
+    unsigned MaxEditDistance = 0,
+    Functor Map = +[](const T &X) -> const T & { return X; }) {
   // The algorithm implemented below is the "classic"
----------------
I'm not sure this `+` is worthwhile - I'd say either make it a non-template entirely, and hardcode this parameter as a function pointer type (that'd work for the two callers here) or make the functor a template parameter and drop the `+` here (so that there's no call indirection overhead). (I guess a third option would be llvm::function_ref for functor-level generality without the template, but somewhat more runtime overhead, probably)

I don't have /super/ strong feelings either way.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126159/new/

https://reviews.llvm.org/D126159