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);
