Introduce get_by_sorted_name

Given that we have several lists of things sorted by name, replace a
bunch of ad-hoc lower_bound calls with a single function.

src/builtin.cpp

@@ -354,7 +354,7 @@ maybe_t<int> builtin_gettext(parser_t &parser, io_streams_t &streams, const wcha
 // Data about all the builtin commands in fish.
 // Functions that are bound to builtin_generic are handled directly by the parser.
 // NOTE: These must be kept in sorted order!
-static const builtin_data_t builtin_datas[] = {
+static constexpr builtin_data_t builtin_datas[] = {
     {L".", &builtin_source, N_(L"Evaluate contents of file")},
     {L":", &builtin_true, N_(L"Return a successful result")},
     {L"[", &builtin_test, N_(L"Test a condition")},
@@ -417,6 +417,7 @@ static const builtin_data_t builtin_datas[] = {
     {L"wait", &builtin_wait, N_(L"Wait for background processes completed")},
     {L"while", &builtin_generic, N_(L"Perform a command multiple times")},
 };
+ASSERT_SORTED_BY_NAME(builtin_datas);
 
 #define BUILTIN_COUNT (sizeof builtin_datas / sizeof *builtin_datas)
 
@@ -429,21 +430,13 @@ static const builtin_data_t builtin_datas[] = {
 ///    Pointer to a builtin_data_t
 ///
 static const builtin_data_t *builtin_lookup(const wcstring &name) {
-    const builtin_data_t *array_end = builtin_datas + BUILTIN_COUNT;
-    const builtin_data_t *found = std::lower_bound(builtin_datas, array_end, name);
-    if (found != array_end && name == found->name) {
-        return found;
-    }
-    return nullptr;
+    return get_by_sorted_name(name.c_str(), builtin_datas);
 }
 
 /// Initialize builtin data.
 void builtin_init() {
     for (size_t i = 0; i < BUILTIN_COUNT; i++) {
-        const wchar_t *name = builtin_datas[i].name;
-        intern_static(name);
-        assert((i == 0 || std::wcscmp(builtin_datas[i - 1].name, name) < 0) &&
-               "builtins are not sorted alphabetically");
+        intern_static(builtin_datas[i].name);
     }
 }
 

src/builtin_string.cpp

@@ -1896,18 +1896,8 @@ maybe_t<int> builtin_string(parser_t &parser, io_streams_t &streams, const wchar
     }
 
     const wchar_t *subcmd_name = argv[1];
-
-    static auto begin = std::begin(string_subcommands);
-    static auto end = std::end(string_subcommands);
-    string_subcommand search{subcmd_name, nullptr};
-    auto binsearch = std::lower_bound(
-        begin, end, search, [&](const string_subcommand &cmd1, const string_subcommand &cmd2) {
-            return wcscmp(cmd1.name, cmd2.name) < 0;
-        });
-    const string_subcommand *subcmd = nullptr;
-    if (binsearch != end && wcscmp(subcmd_name, binsearch->name) == 0) subcmd = &*binsearch;
-
-    if (subcmd == nullptr) {
+    const auto *subcmd = get_by_sorted_name(subcmd_name, string_subcommands);
+    if (!subcmd) {
         streams.err.append_format(BUILTIN_ERR_INVALID_SUBCMD, cmd, subcmd_name);
         builtin_print_error_trailer(parser, streams.err, L"string");
         return STATUS_INVALID_ARGS;

src/common.h

@@ -724,4 +724,22 @@ constexpr bool is_sorted_by_name(const T (&vals)[N], size_t idx = 1) {
 }
 #define ASSERT_SORTED_BY_NAME(x) static_assert(is_sorted_by_name(x), #x " not sorted by name")
 
+/// \return a pointer to the first entry with the given name, assuming the entries are sorted by
+/// name. \return nullptr if not found.
+template <typename T, size_t N>
+const T *get_by_sorted_name(const wchar_t *name, const T (&vals)[N]) {
+    assert(name && "Null name");
+    auto is_less = [](const T &v, const wchar_t *n) -> bool { return std::wcscmp(v.name, n) < 0; };
+    auto where = std::lower_bound(std::begin(vals), std::end(vals), name, is_less);
+    if (where != std::end(vals) && std::wcscmp(where->name, name) == 0) {
+        return &*where;
+    }
+    return nullptr;
+}
+
+template <typename T, size_t N>
+const T *get_by_sorted_name(const wcstring &name, const T (&vals)[N]) {
+    return get_by_sorted_name(name.c_str(), vals);
+}
+
 #endif  // FISH_COMMON_H

src/env.cpp

@@ -110,18 +110,7 @@ static constexpr const electric_var_t electric_variables[] = {
 ASSERT_SORTED_BY_NAME(electric_variables);
 
 const electric_var_t *electric_var_t::for_name(const wchar_t *name) {
-    auto begin = std::begin(electric_variables);
-    auto end = std::end(electric_variables);
-
-    electric_var_t search{name, 0};
-    auto binsearch = std::lower_bound(begin, end, search,
-                                      [&](const electric_var_t &v1, const electric_var_t &v2) {
-                                          return wcscmp(v1.name, v2.name) < 0;
-                                      });
-    if (binsearch != end && wcscmp(name, binsearch->name) == 0) {
-        return &*binsearch;
-    }
-    return nullptr;
+    return get_by_sorted_name(name, electric_variables);
 }
 
 const electric_var_t *electric_var_t::for_name(const wcstring &name) {

src/fish_tests.cpp

@@ -1705,6 +1705,18 @@ static void test_is_sorted_by_name() {
         {L"a"}, {L"aa"}, {L"aaa"}, {L"aaa"}, {L"aaa"}, {L"aazz"}, {L"aazzzz"},
     };
     static_assert(is_sorted_by_name(sorted), "is_sorted_by_name failure");
+    do_test(get_by_sorted_name(L"", sorted) == nullptr);
+    do_test(get_by_sorted_name(L"nope", sorted) == nullptr);
+    do_test(get_by_sorted_name(L"aaaaaaaaaaa", sorted) == nullptr);
+    wcstring last;
+    for (const auto &v : sorted) {
+        // We have multiple items with the same name; only test the first.
+        if (last != v.name) {
+            last = v.name;
+            do_test(get_by_sorted_name(last, sorted) == &v);
+        }
+    }
+
     static constexpr named_t not_sorted[] = {
         {L"a"}, {L"aa"}, {L"aaa"}, {L"q"}, {L"aazz"}, {L"aazz"}, {L"aazz"}, {L"aazzzz"},
     };

src/input.cpp

@@ -920,16 +920,8 @@ const wcstring_list_t &input_function_get_names() {
 maybe_t<readline_cmd_t> input_function_get_code(const wcstring &name) {
     // `input_function_metadata` is required to be kept in asciibetical order, making it OK to do
     // a binary search for the matching name.
-    constexpr auto end = &input_function_metadata[0] + input_function_count;
-    auto result = std::lower_bound(
-        &input_function_metadata[0], end,
-        input_function_metadata_t{name.data(), static_cast<readline_cmd_t>(-1)},
-        [&](const input_function_metadata_t &lhs, const input_function_metadata_t &rhs) {
-            return wcscmp(lhs.name, rhs.name) < 0;
-        });
-
-    if (result != end && result->name[0] && name == result->name) {
-        return result->code;
+    if (const input_function_metadata_t *md = get_by_sorted_name(name, input_function_metadata)) {
+        return md->code;
     }
     return none();
 }