Add Target::builds_swift_module() method.
The SwiftValues::builds_module() method would always return false before
SwiftValues::OnResolvedTarget() is called, and this masked a subtle bug
when performing target resolution (see below).
This CL fixes the issue by adding a new method to the Target class
(Target::builds_swift_module()). This is necessary since giving the
correct answer requires knowing which Target instance the SwiftValue
class is associated with. Call sites are updated to use the new method.
Regarding the bug itself:
- During a Target::OnResolved() call, the PullRecursiveHardDeps()
method is called before SwiftValues::OnTargetResolved().
- The PullRecursiveHardDeps() calls the `hard_dep()` method on the
current target, and its dependencies as in:
```
for (const auto& pair : GetDeps(DEPS_LINKED)) {
// Direct hard dependencies.
if (hard_dep() || pair.ptr->hard_dep()) {
recursive_hard_deps_.insert(pair.ptr);
continue;
}
...
```
If the current target generates a Swift module, the first `hard_dep()`
call would always mistakenly return false (because
SwiftValues::OnTargetResolved() has not been called yet).
Note that the `pair.ptr->hard_dep()` would always return the correct
value since all dependencies of the current target are fully resolved
when this code runs (and hence their own
SwiftValues::OnTargetResolved() call had been performed).
The CL ensures that the first `hard_dep()` call always returns the
correct value. This also fixes a unit-test that was expecting the
previous behaviour.
Bug: None
Change-Id: I6ce8f2f182e22666282d0f7b889fa8df94c45c48
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/13600
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: David Turner <digit@google.com>
diff --git a/src/gn/ninja_binary_target_writer.cc b/src/gn/ninja_binary_target_writer.cc
index e6f0cda..e3e36e5 100644
--- a/src/gn/ninja_binary_target_writer.cc
+++ b/src/gn/ninja_binary_target_writer.cc
@@ -156,7 +156,7 @@
// don't link at all.
bool can_link_libs = target_->IsFinal();
- if (can_link_libs && dep->swift_values().builds_module())
+ if (can_link_libs && dep->builds_swift_module())
classified_deps->swiftmodule_deps.push_back(dep);
if (target_->source_types_used().RustSourceUsed() &&
diff --git a/src/gn/ninja_c_binary_target_writer.cc b/src/gn/ninja_c_binary_target_writer.cc
index 052f03a..2a13c86 100644
--- a/src/gn/ninja_c_binary_target_writer.cc
+++ b/src/gn/ninja_c_binary_target_writer.cc
@@ -678,7 +678,7 @@
swiftmodules.push_back(dep->swift_values().module_output_file());
implicit_deps.push_back(dep->swift_values().module_output_file());
}
- if (target_->swift_values().builds_module()) {
+ if (target_->builds_swift_module()) {
swiftmodules.push_back(target_->swift_values().module_output_file());
implicit_deps.push_back(target_->swift_values().module_output_file());
}
diff --git a/src/gn/ninja_c_binary_target_writer_unittest.cc b/src/gn/ninja_c_binary_target_writer_unittest.cc
index ff51121..6056c36 100644
--- a/src/gn/ninja_c_binary_target_writer_unittest.cc
+++ b/src/gn/ninja_c_binary_target_writer_unittest.cc
@@ -2378,10 +2378,10 @@
"target_output_name = bar\n"
"\n"
"build obj/bar/Bar.swiftmodule: swift ../../bar/bar.swift"
- " || obj/foo/foo.stamp\n"
+ " || obj/bar/group.stamp obj/foo/foo.stamp\n"
"\n"
"build obj/bar/bar.o: stamp obj/bar/Bar.swiftmodule"
- " || obj/foo/foo.stamp\n"
+ " || obj/bar/group.stamp obj/foo/foo.stamp\n"
"\n"
"build obj/bar/bar.stamp: stamp obj/bar/bar.o "
"|| obj/bar/group.stamp obj/foo/foo.stamp\n";
diff --git a/src/gn/swift_values.cc b/src/gn/swift_values.cc
index a70c72f..3cfd377 100644
--- a/src/gn/swift_values.cc
+++ b/src/gn/swift_values.cc
@@ -48,7 +48,7 @@
pair.ptr->swift_values().public_modules().end());
}
- if (target->swift_values().builds_module())
+ if (target->builds_swift_module())
target->swift_values().public_modules_.push_back(target);
}
diff --git a/src/gn/swift_values.h b/src/gn/swift_values.h
index 3852366..91ec066 100644
--- a/src/gn/swift_values.h
+++ b/src/gn/swift_values.h
@@ -35,9 +35,6 @@
std::string& module_name() { return module_name_; }
const std::string module_name() const { return module_name_; }
- // Returns whether the target generates a .swiftmodule.
- bool builds_module() const { return !module_output_file_.value().empty(); }
-
// Name of the generated .swiftmodule file. Computed when the target
// is resolved.
const OutputFile& module_output_file() const { return module_output_file_; }
diff --git a/src/gn/target.cc b/src/gn/target.cc
index f0c897e..7b545b5 100644
--- a/src/gn/target.cc
+++ b/src/gn/target.cc
@@ -889,7 +889,7 @@
// by the current target).
if (pair.ptr->IsBinary() && !pair.ptr->all_headers_public() &&
pair.ptr->public_headers().empty() &&
- !pair.ptr->swift_values().builds_module()) {
+ !pair.ptr->builds_swift_module()) {
continue;
}
diff --git a/src/gn/target.h b/src/gn/target.h
index de076a8..66e5c6c 100644
--- a/src/gn/target.h
+++ b/src/gn/target.h
@@ -239,7 +239,7 @@
return output_type_ == ACTION || output_type_ == ACTION_FOREACH ||
output_type_ == COPY_FILES || output_type_ == CREATE_BUNDLE ||
output_type_ == BUNDLE_DATA || output_type_ == GENERATED_FILE ||
- (IsBinary() && has_swift_values() && swift_values().builds_module());
+ builds_swift_module();
}
// Returns the iterator range which can be used in range-based for loops
@@ -311,6 +311,12 @@
const SwiftValues& swift_values() const;
bool has_swift_values() const { return swift_values_.get(); }
+ // Return true if this targets builds a SwiftModule
+ bool builds_swift_module() const {
+ return IsBinary() && has_swift_values() &&
+ source_types_used().SwiftSourceUsed();
+ }
+
RustValues& rust_values();
const RustValues& rust_values() const;
bool has_rust_values() const { return rust_values_.get(); }
