Skip attempting to resolve known files as module names. This is part of a larger process to improve the performance of suggestions, but does not have change the time complexity until gn-review.googlesource.com/c/gn/+/26242 is submitted (benchmarks in that commit) Change-Id: I6e0067ca86f1e1afc6c8a828004011fd6a6a6964 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/26260 Reviewed-by: Takuto Ikuta <tikuta@google.com> Commit-Queue: Matt Stark <msta@google.com>
diff --git a/src/gn/command_check.cc b/src/gn/command_check.cc index d28a070..dbd322c 100644 --- a/src/gn/command_check.cc +++ b/src/gn/command_check.cc
@@ -316,7 +316,7 @@ [&](std::string_view str, TextDecoration dec, HtmlEscaping esc) { buf.emplace_back(str, dec, esc); }, - cache, apply, setup); + cache, /*must_be_file=*/true, apply, setup); fixed = apply && (exit_code == SuggestResult::kSuccess); if (!buf.empty()) { has_suggestions = true;
diff --git a/src/gn/command_suggest.cc b/src/gn/command_suggest.cc index aafc719..2aadfdd 100644 --- a/src/gn/command_suggest.cc +++ b/src/gn/command_suggest.cc
@@ -320,6 +320,7 @@ const std::vector<const Target*>& all_targets, const Label& current_toolchain, std::string_view input, + bool must_be_file, TargetResolutionCache& cache, const Target* includer) { auto sort_results = [](auto& vec) { @@ -328,38 +329,40 @@ }); }; std::vector<std::pair<const Target*, commands::ApiScope>> results; - std::string_view module_name = input; - commands::ApiScope is_private = commands::ApiScope::kPublic; - if (module_name.ends_with(kPrivateSuffix)) { - is_private = commands::ApiScope::kPrivate; - module_name.remove_suffix(kPrivateSuffix.size()); - } - - // Try to resolve as a module name. - for (const Target* target : all_targets) { - if (target->module_name() == module_name) { - results.emplace_back(target, is_private); + if (!must_be_file) { + std::string_view module_name = input; + commands::ApiScope is_private = commands::ApiScope::kPublic; + if (module_name.ends_with(kPrivateSuffix)) { + is_private = commands::ApiScope::kPrivate; + module_name.remove_suffix(kPrivateSuffix.size()); } - } - if (!results.empty()) { - sort_results(results); - return {results, true}; - } - // If that doesn't work, try to resolve as an absolute target label. - if (input.starts_with("//") && input.find(':') != std::string_view::npos) { - Err err; - Label want; - Value input_value(nullptr, std::string(input)); - want = Label::Resolve(SourceDir("//"), build_settings->root_path_utf8(), - current_toolchain, input_value, &err); - if (!err.has_error()) { - for (const Target* target : all_targets) { - if (target->label() == want) { - results.emplace_back(target, is_private); - // We know each label corresponds to exactly one target, so we don't - // need to keep going. - return {results, true}; + // Try to resolve as a module name. + for (const Target* target : all_targets) { + if (target->module_name() == module_name) { + results.emplace_back(target, is_private); + } + } + if (!results.empty()) { + sort_results(results); + return {results, true}; + } + + // If that doesn't work, try to resolve as an absolute target label. + if (input.starts_with("//") && input.find(':') != std::string_view::npos) { + Err err; + Label want; + Value input_value(nullptr, std::string(input)); + want = Label::Resolve(SourceDir("//"), build_settings->root_path_utf8(), + current_toolchain, input_value, &err); + if (!err.has_error()) { + for (const Target* target : all_targets) { + if (target->label() == want) { + results.emplace_back(target, is_private); + // We know each label corresponds to exactly one target, so we don't + // need to keep going. + return {results, true}; + } } } } @@ -402,6 +405,7 @@ std::string_view included_name, OutputStringFunc output_fn, TargetResolutionCache& cache, + bool must_be_file, bool apply, Setup* setup) { if (apply) { @@ -547,8 +551,8 @@ auto ResolveSuggestion = [&](std::string_view value, const Target* target_context = nullptr) { const auto& [targets, ok] = ResolveSuggestionToTarget( - build_settings, all_targets, current_toolchain, value, cache, - target_context); + build_settings, all_targets, current_toolchain, value, must_be_file, + cache, target_context); if (!ok) { StartError(); if (value.starts_with("//")) { @@ -996,7 +1000,7 @@ has_suggestions = true; ::OutputString(str, dec, esc); }, - cache, apply, setup); + cache, /*must_be_file=*/false, apply, setup); if (res == SuggestResult::kFailure) { exit_status = SuggestResult::kFailure; } else if (res == SuggestResult::kUnapplied &&
diff --git a/src/gn/command_suggest_unittest.cc b/src/gn/command_suggest_unittest.cc index 68ee9c5..cf8c876 100644 --- a/src/gn/command_suggest_unittest.cc +++ b/src/gn/command_suggest_unittest.cc
@@ -110,7 +110,7 @@ { auto [results, ok] = commands::ResolveSuggestionToTarget( setup_scope.build_settings(), all_targets, default_toolchain, - "my_module", cache); + "my_module", /*must_be_file=*/false, cache); std::vector<std::pair<const Target*, commands::ApiScope>> expected = { {&target, commands::ApiScope::kPublic}}; EXPECT_EQ(expected, results); @@ -121,7 +121,7 @@ { auto [results, ok] = commands::ResolveSuggestionToTarget( setup_scope.build_settings(), all_targets, default_toolchain, - "my_module_Private", cache); + "my_module_Private", /*must_be_file=*/false, cache); std::vector<std::pair<const Target*, commands::ApiScope>> expected = { {&target, commands::ApiScope::kPrivate}}; EXPECT_EQ(expected, results); @@ -148,7 +148,8 @@ // Test resolving "//:hello" auto [results_label, ok_label] = commands::ResolveSuggestionToTarget( setup_scope.build_settings(), all_targets, - setup_scope.toolchain()->label(), "//:hello", cache); + setup_scope.toolchain()->label(), "//:hello", /*must_be_file=*/false, + cache); std::vector<std::pair<const Target*, commands::ApiScope>> expected_label = { {&target, commands::ApiScope::kPublic}}; @@ -158,7 +159,7 @@ // Test resolving "//:hello(//build/toolchain:gcc)" auto [results_toolchain, ok_toolchain] = commands::ResolveSuggestionToTarget( setup_scope.build_settings(), all_targets, default_toolchain, - "//:hello(//build/toolchain:gcc)", cache); + "//:hello(//build/toolchain:gcc)", /*must_be_file=*/false, cache); std::vector<std::pair<const Target*, commands::ApiScope>> expected_toolchain = {{&target_gcc, commands::ApiScope::kPublic}}; @@ -267,7 +268,7 @@ { auto [results, ok] = commands::ResolveSuggestionToTarget( setup_scope.build_settings(), all_targets, current_toolchain, - "//public.h", cache); + "//public.h", /*must_be_file=*/true, cache); std::vector<std::pair<const Target*, commands::ApiScope>> expected = { {&explicit_target, commands::ApiScope::kPublic}}; EXPECT_TRUE(ok); @@ -277,7 +278,7 @@ { auto [results, ok] = commands::ResolveSuggestionToTarget( setup_scope.build_settings(), all_targets, current_toolchain, - "../../private.h", cache); + "../../private.h", /*must_be_file=*/true, cache); std::vector<std::pair<const Target*, commands::ApiScope>> expected = { {&explicit_target, commands::ApiScope::kPrivate}}; EXPECT_TRUE(ok); @@ -287,7 +288,7 @@ { auto [results, ok] = commands::ResolveSuggestionToTarget( setup_scope.build_settings(), all_targets, current_toolchain, - "//implicit_public.h", cache); + "//implicit_public.h", /*must_be_file=*/true, cache); std::vector<std::pair<const Target*, commands::ApiScope>> expected = { {&implicit_target, commands::ApiScope::kPublic}}; EXPECT_TRUE(ok); @@ -297,14 +298,14 @@ { auto [results, ok] = commands::ResolveSuggestionToTarget( setup_scope.build_settings(), all_targets, current_toolchain, - "nonexistent_file.h", cache); + "nonexistent_file.h", /*must_be_file=*/true, cache); EXPECT_FALSE(ok); } { auto [results, ok] = commands::ResolveSuggestionToTarget( setup_scope.build_settings(), all_targets, current_toolchain, - "//out/Debug/generated_file.h", cache); + "//out/Debug/generated_file.h", /*must_be_file=*/true, cache); std::vector<std::pair<const Target*, commands::ApiScope>> expected = { {&generated, commands::ApiScope::kPublic}}; EXPECT_TRUE(ok); @@ -316,7 +317,7 @@ { auto [results, ok] = commands::ResolveSuggestionToTarget( setup_scope.build_settings(), all_targets, current_toolchain, - "//out/Debug/generated_file.h", consumer_cache); + "//out/Debug/generated_file.h", /*must_be_file=*/true, consumer_cache); std::vector<std::pair<const Target*, commands::ApiScope>> expected = { {&consumer, commands::ApiScope::kPublic}}; EXPECT_TRUE(ok); @@ -326,7 +327,7 @@ { auto [results, ok] = commands::ResolveSuggestionToTarget( setup_scope.build_settings(), all_targets, current_toolchain, - "//no_target.h", consumer_cache); + "//no_target.h", /*must_be_file=*/true, consumer_cache); std::vector<std::pair<const Target*, commands::ApiScope>> expected_targets; EXPECT_TRUE(ok); EXPECT_EQ(expected_targets, results); @@ -335,7 +336,7 @@ { auto [results, ok] = commands::ResolveSuggestionToTarget( setup_scope.build_settings(), all_targets, current_toolchain, - "//default_toolchain.h", consumer_cache); + "//default_toolchain.h", /*must_be_file=*/true, consumer_cache); std::vector<std::pair<const Target*, commands::ApiScope>> expected_targets = { {&simple_secondary, commands::ApiScope::kPublic}, @@ -348,7 +349,7 @@ { auto [results, ok] = commands::ResolveSuggestionToTarget( setup_scope.build_settings(), all_targets, current_toolchain, - "//secondary_toolchain.h", consumer_cache); + "//secondary_toolchain.h", /*must_be_file=*/true, consumer_cache); std::vector<std::pair<const Target*, commands::ApiScope>> expected_targets = {{{&simple_secondary, commands::ApiScope::kPublic}}}; EXPECT_TRUE(ok); @@ -358,7 +359,7 @@ { auto [results, ok] = commands::ResolveSuggestionToTarget( setup_scope.build_settings(), all_targets, current_toolchain, - "my_header.h", consumer_cache, &consumer); + "my_header.h", /*must_be_file=*/true, consumer_cache, &consumer); EXPECT_TRUE(ok); std::vector<std::pair<const Target*, commands::ApiScope>> expected_targets = {{{&included_target, commands::ApiScope::kPublic}}}; @@ -602,7 +603,7 @@ commands::SuggestResult result = commands::OutputSuggestions( project.targets(), &project.setup.build_settings(), project.default_toolchain(), "//includer.cc", "//included.h", collect, - cache, true, &project.setup); + cache, /*must_be_file=*/false, /*apply=*/true, &project.setup); EXPECT_EQ(commands::SuggestResult::kSuccess, result); EXPECT_EQ(
diff --git a/src/gn/commands.h b/src/gn/commands.h index 1a362bb..471ab2f 100644 --- a/src/gn/commands.h +++ b/src/gn/commands.h
@@ -159,6 +159,7 @@ std::string_view included_name, OutputStringFunc output_fn, TargetResolutionCache& cache, + bool must_be_file = false, bool apply = false, Setup* setup = nullptr); @@ -337,6 +338,7 @@ const std::vector<const Target*>& all_targets, const Label& current_toolchain, std::string_view input, + bool must_be_file, TargetResolutionCache& cache, const Target* includer = nullptr);