Add ResolvedTargetData::GetLinkedLibraries() This CL removes Target::all_libs() and Target::all_lib_dirs() and move their computation to the ResolvedTargetData class, where the values will be generated on demand and returned by the GetLinkedLibraries() and GetLinkedLibraryDirs() methods respectively. Bug: 331 Change-Id: I4ce91fca247ac4bbe8a51666f0f58fa55bfe925b Reviewed-on: https://gn-review.googlesource.com/c/gn/+/15323 Commit-Queue: David Turner <digit@google.com> Reviewed-by: Takuto Ikuta <tikuta@google.com>
diff --git a/src/gn/desc_builder.cc b/src/gn/desc_builder.cc index 444a5e0..f82c079 100644 --- a/src/gn/desc_builder.cc +++ b/src/gn/desc_builder.cc
@@ -14,6 +14,7 @@ #include "gn/desc_builder.h" #include "gn/input_file.h" #include "gn/parse_tree.h" +#include "gn/resolved_target_data.h" #include "gn/runtime_deps.h" #include "gn/rust_variables.h" #include "gn/scope.h" @@ -582,10 +583,12 @@ // currently implement a blame feature for this since the bottom-up // inheritance makes this difficult. + ResolvedTargetData resolved; + // Libs can be part of any target and get recursively pushed up the chain, // so display them regardless of target type. if (what(variables::kLibs)) { - const UniqueVector<LibFile>& all_libs = target_->all_libs(); + const auto& all_libs = resolved.GetLinkedLibraries(target_); if (!all_libs.empty()) { auto libs = std::make_unique<base::ListValue>(); for (size_t i = 0; i < all_libs.size(); i++) @@ -595,7 +598,7 @@ } if (what(variables::kLibDirs)) { - const UniqueVector<SourceDir>& all_lib_dirs = target_->all_lib_dirs(); + const auto& all_lib_dirs = resolved.GetLinkedLibraryDirs(target_); if (!all_lib_dirs.empty()) { auto lib_dirs = std::make_unique<base::ListValue>(); for (size_t i = 0; i < all_lib_dirs.size(); i++)
diff --git a/src/gn/ninja_binary_target_writer.cc b/src/gn/ninja_binary_target_writer.cc index b9ecc21..d9d403f 100644 --- a/src/gn/ninja_binary_target_writer.cc +++ b/src/gn/ninja_binary_target_writer.cc
@@ -327,7 +327,7 @@ const Tool* tool) { // Write library search paths that have been recursively pushed // through the dependency tree. - const UniqueVector<SourceDir>& all_lib_dirs = target_->all_lib_dirs(); + const auto& all_lib_dirs = resolved().GetLinkedLibraryDirs(target_); if (!all_lib_dirs.empty()) { // Since we're passing these on the command line to the linker and not // to Ninja, we need to do shell escaping. @@ -380,7 +380,7 @@ ESCAPE_NINJA_COMMAND); EscapeOptions lib_escape_opts; lib_escape_opts.mode = ESCAPE_NINJA_COMMAND; - const UniqueVector<LibFile>& all_libs = target_->all_libs(); + const auto& all_libs = resolved().GetLinkedLibraries(target_); for (size_t i = 0; i < all_libs.size(); i++) { const LibFile& lib_file = all_libs[i]; const std::string& lib_value = lib_file.value();
diff --git a/src/gn/ninja_c_binary_target_writer.cc b/src/gn/ninja_c_binary_target_writer.cc index c0f73b8..f74d208 100644 --- a/src/gn/ninja_c_binary_target_writer.cc +++ b/src/gn/ninja_c_binary_target_writer.cc
@@ -642,7 +642,7 @@ } // Libraries specified by paths. - for (const auto& lib : target_->all_libs()) { + for (const auto& lib : resolved().GetLinkedLibraries(target_)) { if (lib.is_source_file()) { implicit_deps.push_back( OutputFile(settings_->build_settings(), lib.source_file()));
diff --git a/src/gn/resolved_target_data.cc b/src/gn/resolved_target_data.cc index 742a4cc..722858d 100644 --- a/src/gn/resolved_target_data.cc +++ b/src/gn/resolved_target_data.cc
@@ -4,6 +4,8 @@ #include "gn/resolved_target_data.h" +#include "gn/config_values_extractors.h" + ResolvedTargetData::TargetInfo* ResolvedTargetData::GetTargetInfo( const Target* target) const { auto ret = targets_.PushBackWithIndex(target); @@ -12,3 +14,25 @@ } return infos_[ret.second].get(); } + +void ResolvedTargetData::ComputeLibInfo(TargetInfo* info) const { + UniqueVector<SourceDir> all_lib_dirs; + UniqueVector<LibFile> all_libs; + + for (ConfigValuesIterator iter(info->target); !iter.done(); iter.Next()) { + const ConfigValues& cur = iter.cur(); + all_lib_dirs.Append(cur.lib_dirs()); + all_libs.Append(cur.libs()); + } + for (const Target* dep : info->deps.linked_deps()) { + if (!dep->IsFinal() || dep->output_type() == Target::STATIC_LIBRARY) { + const TargetInfo* dep_info = GetTargetLibInfo(dep); + all_lib_dirs.Append(dep_info->lib_dirs); + all_libs.Append(dep_info->libs); + } + } + + info->lib_dirs = all_lib_dirs.release(); + info->libs = all_libs.release(); + info->has_lib_info = true; +}
diff --git a/src/gn/resolved_target_data.h b/src/gn/resolved_target_data.h index 510d6f6..13d7797 100644 --- a/src/gn/resolved_target_data.h +++ b/src/gn/resolved_target_data.h
@@ -60,6 +60,21 @@ return GetTargetDeps(target).linked_deps(); } + // The list of all library directory search path to add to the final link + // command of linkable binary. For example, if this returns ['dir1', 'dir2'] + // a command for a C++ linker would typically use `-Ldir1 -Ldir2`. + const std::vector<SourceDir>& GetLinkedLibraryDirs( + const Target* target) const { + return GetTargetLibInfo(target)->lib_dirs; + } + + // The list of all library files to add to the final link command of linkable + // binaries. For example, if this returns ['foo', '/path/to/bar'], the command + // for a C++ linker would typically use '-lfoo /path/to/bar'. + const std::vector<LibFile>& GetLinkedLibraries(const Target* target) const { + return GetTargetLibInfo(target)->libs; + } + private: // The information associated with a given Target pointer. struct TargetInfo { @@ -73,12 +88,32 @@ const Target* target = nullptr; ResolvedTargetDeps deps; + + bool has_lib_info = false; + + // Only valid if |has_lib_info| is true. + std::vector<SourceDir> lib_dirs; + std::vector<LibFile> libs; }; // Retrieve TargetInfo value associated with |target|. Create // a new empty instance on demand if none is already available. TargetInfo* GetTargetInfo(const Target* target) const; + const TargetInfo* GetTargetLibInfo(const Target* target) const { + TargetInfo* info = GetTargetInfo(target); + if (!info->has_lib_info) { + ComputeLibInfo(info); + DCHECK(info->has_lib_info); + } + return info; + } + + // Compute the portion of TargetInfo guarded by one of the |has_xxx| + // booleans. This performs recursive and expensive computations and + // should only be called once per TargetInfo instance. + void ComputeLibInfo(TargetInfo* info) const; + // A { Target* -> TargetInfo } map that will create entries // on demand (hence the mutable qualifier). Implemented with a // UniqueVector<> and a parallel vector of unique TargetInfo
diff --git a/src/gn/resolved_target_data_unittest.cc b/src/gn/resolved_target_data_unittest.cc index 03cd28b..440c604 100644 --- a/src/gn/resolved_target_data_unittest.cc +++ b/src/gn/resolved_target_data_unittest.cc
@@ -51,3 +51,61 @@ EXPECT_EQ(b_deps.public_deps().size(), 0u); EXPECT_EQ(b_deps.data_deps().size(), 0u); } + +// Tests that lib[_dir]s are inherited across deps boundaries for static +// libraries but not executables. +TEST(ResolvedTargetDataTest, LibInheritance) { + TestWithScope setup; + Err err; + + ResolvedTargetData resolved; + + const LibFile lib("foo"); + const SourceDir libdir("/foo_dir/"); + + // Leaf target with ldflags set. + TestTarget z(setup, "//foo:z", Target::STATIC_LIBRARY); + z.config_values().libs().push_back(lib); + z.config_values().lib_dirs().push_back(libdir); + ASSERT_TRUE(z.OnResolved(&err)); + + // All lib[_dir]s should be set when target is resolved. + const auto& all_libs = resolved.GetLinkedLibraries(&z); + ASSERT_EQ(1u, all_libs.size()); + EXPECT_EQ(lib, all_libs[0]); + + const auto& all_lib_dirs = resolved.GetLinkedLibraryDirs(&z); + ASSERT_EQ(1u, all_lib_dirs.size()); + EXPECT_EQ(libdir, all_lib_dirs[0]); + + // Shared library target should inherit the libs from the static library + // and its own. Its own flag should be before the inherited one. + const LibFile second_lib("bar"); + const SourceDir second_libdir("/bar_dir/"); + TestTarget shared(setup, "//foo:shared", Target::SHARED_LIBRARY); + shared.config_values().libs().push_back(second_lib); + shared.config_values().lib_dirs().push_back(second_libdir); + shared.private_deps().push_back(LabelTargetPair(&z)); + ASSERT_TRUE(shared.OnResolved(&err)); + + const auto& all_libs2 = resolved.GetLinkedLibraries(&shared); + ASSERT_EQ(2u, all_libs2.size()); + EXPECT_EQ(second_lib, all_libs2[0]); + EXPECT_EQ(lib, all_libs2[1]); + + const auto& all_lib_dirs2 = resolved.GetLinkedLibraryDirs(&shared); + ASSERT_EQ(2u, all_lib_dirs2.size()); + EXPECT_EQ(second_libdir, all_lib_dirs2[0]); + EXPECT_EQ(libdir, all_lib_dirs2[1]); + + // Executable target shouldn't get either by depending on shared. + TestTarget exec(setup, "//foo:exec", Target::EXECUTABLE); + exec.private_deps().push_back(LabelTargetPair(&shared)); + ASSERT_TRUE(exec.OnResolved(&err)); + + const auto& all_libs3 = resolved.GetLinkedLibraries(&exec); + EXPECT_EQ(0u, all_libs3.size()); + + const auto& all_lib_dirs3 = resolved.GetLinkedLibraryDirs(&exec); + EXPECT_EQ(0u, all_lib_dirs3.size()); +}
diff --git a/src/gn/target.cc b/src/gn/target.cc index 2cebcf7..4d8cc56 100644 --- a/src/gn/target.cc +++ b/src/gn/target.cc
@@ -493,9 +493,6 @@ // order (local ones first, then the dependency's). for (ConfigValuesIterator iter(this); !iter.done(); iter.Next()) { const ConfigValues& cur = iter.cur(); - all_lib_dirs_.Append(cur.lib_dirs().begin(), cur.lib_dirs().end()); - all_libs_.Append(cur.libs().begin(), cur.libs().end()); - all_framework_dirs_.Append(cur.framework_dirs().begin(), cur.framework_dirs().end()); all_frameworks_.Append(cur.frameworks().begin(), cur.frameworks().end()); @@ -862,9 +859,6 @@ // Library settings are always inherited across static library boundaries. if (!dep->IsFinal() || dep->output_type() == STATIC_LIBRARY) { - all_lib_dirs_.Append(dep->all_lib_dirs()); - all_libs_.Append(dep->all_libs()); - all_framework_dirs_.Append(dep->all_framework_dirs()); all_frameworks_.Append(dep->all_frameworks()); all_weak_frameworks_.Append(dep->all_weak_frameworks());
diff --git a/src/gn/target.h b/src/gn/target.h index fbdc6c8..c6f93c4 100644 --- a/src/gn/target.h +++ b/src/gn/target.h
@@ -332,9 +332,6 @@ return rust_transitive_inheritable_libs_; } - const UniqueVector<SourceDir>& all_lib_dirs() const { return all_lib_dirs_; } - const UniqueVector<LibFile>& all_libs() const { return all_libs_; } - const UniqueVector<SourceDir>& all_framework_dirs() const { return all_framework_dirs_; } @@ -504,11 +501,6 @@ // that need to be linked. InheritedLibraries inherited_libraries_; - // These libs and dirs are inherited from statically linked deps and all - // configs applying to this target. - UniqueVector<SourceDir> all_lib_dirs_; - UniqueVector<LibFile> all_libs_; - // These frameworks and dirs are inherited from statically linked deps and // all configs applying to this target. UniqueVector<SourceDir> all_framework_dirs_;
diff --git a/src/gn/target_unittest.cc b/src/gn/target_unittest.cc index 005335d..7a3ca6e 100644 --- a/src/gn/target_unittest.cc +++ b/src/gn/target_unittest.cc
@@ -9,6 +9,7 @@ #include "gn/build_settings.h" #include "gn/config.h" +#include "gn/resolved_target_data.h" #include "gn/scheduler.h" #include "gn/settings.h" #include "gn/test_with_scheduler.h" @@ -36,52 +37,6 @@ using TargetTest = TestWithScheduler; -// Tests that lib[_dir]s are inherited across deps boundaries for static -// libraries but not executables. -TEST_F(TargetTest, LibInheritance) { - TestWithScope setup; - Err err; - - const LibFile lib("foo"); - const SourceDir libdir("/foo_dir/"); - - // Leaf target with ldflags set. - TestTarget z(setup, "//foo:z", Target::STATIC_LIBRARY); - z.config_values().libs().push_back(lib); - z.config_values().lib_dirs().push_back(libdir); - ASSERT_TRUE(z.OnResolved(&err)); - - // All lib[_dir]s should be set when target is resolved. - ASSERT_EQ(1u, z.all_libs().size()); - EXPECT_EQ(lib, z.all_libs()[0]); - ASSERT_EQ(1u, z.all_lib_dirs().size()); - EXPECT_EQ(libdir, z.all_lib_dirs()[0]); - - // Shared library target should inherit the libs from the static library - // and its own. Its own flag should be before the inherited one. - const LibFile second_lib("bar"); - const SourceDir second_libdir("/bar_dir/"); - TestTarget shared(setup, "//foo:shared", Target::SHARED_LIBRARY); - shared.config_values().libs().push_back(second_lib); - shared.config_values().lib_dirs().push_back(second_libdir); - shared.private_deps().push_back(LabelTargetPair(&z)); - ASSERT_TRUE(shared.OnResolved(&err)); - - ASSERT_EQ(2u, shared.all_libs().size()); - EXPECT_EQ(second_lib, shared.all_libs()[0]); - EXPECT_EQ(lib, shared.all_libs()[1]); - ASSERT_EQ(2u, shared.all_lib_dirs().size()); - EXPECT_EQ(second_libdir, shared.all_lib_dirs()[0]); - EXPECT_EQ(libdir, shared.all_lib_dirs()[1]); - - // Executable target shouldn't get either by depending on shared. - TestTarget exec(setup, "//foo:exec", Target::EXECUTABLE); - exec.private_deps().push_back(LabelTargetPair(&shared)); - ASSERT_TRUE(exec.OnResolved(&err)); - EXPECT_EQ(0u, exec.all_libs().size()); - EXPECT_EQ(0u, exec.all_lib_dirs().size()); -} - // Tests that framework[_dir]s are inherited across deps boundaries for static // libraries but not executables. TEST_F(TargetTest, FrameworkInheritance) { @@ -366,10 +321,14 @@ EXPECT_EQ(&b, a_inherited[0]); // A should inherit the libs and lib_dirs from the C. - ASSERT_EQ(1u, a.all_libs().size()); - EXPECT_EQ(lib, a.all_libs()[0]); - ASSERT_EQ(1u, a.all_lib_dirs().size()); - EXPECT_EQ(lib_dir, a.all_lib_dirs()[0]); + ResolvedTargetData resolved; + const auto& a_all_libs = resolved.GetLinkedLibraries(&a); + ASSERT_EQ(1u, a_all_libs.size()); + EXPECT_EQ(lib, a_all_libs[0]); + + const auto& a_all_lib_dirs = resolved.GetLinkedLibraryDirs(&a); + ASSERT_EQ(1u, a_all_lib_dirs.size()); + EXPECT_EQ(lib_dir, a_all_lib_dirs[0]); } TEST_F(TargetTest, InheritCompleteStaticLibStaticLibDeps) { @@ -684,8 +643,10 @@ // Libs have special handling, check that they were forwarded from the // public config to all_libs. - ASSERT_EQ(1u, dep_on_pub.all_libs().size()); - ASSERT_EQ(lib_name, dep_on_pub.all_libs()[0]); + ResolvedTargetData resolved; + const auto& dep_on_pub_all_libs = resolved.GetLinkedLibraries(&dep_on_pub); + ASSERT_EQ(1u, dep_on_pub_all_libs.size()); + ASSERT_EQ(lib_name, dep_on_pub_all_libs[0]); // This target has a private dependency on dest for forwards configs. TestTarget forward(setup, "//a:f", Target::SOURCE_SET);