Add structure to return classified deps Change NinjaBinaryTargetWriter::GetDeps() to return its output via a structure (instead of using multiple output parameters) and also rename the method to GetClassifiedDeps(). Using a structure to return the output will make it easier to add other kinds of deps (notably swift requires a new type of deps for the .swiftmodule files). Bug: 121 Change-Id: I4c8f479f6619e0377244955331e265b03548af65 Reviewed-on: https://gn-review.googlesource.com/c/gn/+/9540 Reviewed-by: Brett Wilson <brettw@chromium.org> Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
diff --git a/src/gn/ninja_binary_target_writer.cc b/src/gn/ninja_binary_target_writer.cc index da7733b..098a595 100644 --- a/src/gn/ninja_binary_target_writer.cc +++ b/src/gn/ninja_binary_target_writer.cc
@@ -108,53 +108,44 @@ // depend on this will reference the object files directly. However, writing // this rule allows the user to type the name of the target and get a build // which can be convenient for development. - UniqueVector<OutputFile> extra_object_files; - UniqueVector<const Target*> linkable_deps; - UniqueVector<const Target*> non_linkable_deps; - UniqueVector<const Target*> framework_deps; - GetDeps(&extra_object_files, &linkable_deps, &non_linkable_deps, - &framework_deps); + ClassifiedDeps classified_deps = GetClassifiedDeps(); // The classifier should never put extra object files in a source sets: any // source sets that we depend on should appear in our non-linkable deps // instead. - DCHECK(extra_object_files.empty()); + DCHECK(classified_deps.extra_object_files.empty()); std::vector<OutputFile> order_only_deps; - for (auto* dep : non_linkable_deps) + for (auto* dep : classified_deps.non_linkable_deps) order_only_deps.push_back(dep->dependency_output_file()); WriteStampForTarget(object_files, order_only_deps); } -void NinjaBinaryTargetWriter::GetDeps( - UniqueVector<OutputFile>* extra_object_files, - UniqueVector<const Target*>* linkable_deps, - UniqueVector<const Target*>* non_linkable_deps, - UniqueVector<const Target*>* framework_deps) const { +NinjaBinaryTargetWriter::ClassifiedDeps +NinjaBinaryTargetWriter::GetClassifiedDeps() const { + ClassifiedDeps classified_deps; + // Normal public/private deps. for (const auto& pair : target_->GetDeps(Target::DEPS_LINKED)) { - ClassifyDependency(pair.ptr, extra_object_files, linkable_deps, - non_linkable_deps, framework_deps); + ClassifyDependency(pair.ptr, &classified_deps); } // Inherited libraries. for (auto* inherited_target : target_->inherited_libraries().GetOrdered()) { - ClassifyDependency(inherited_target, extra_object_files, linkable_deps, - non_linkable_deps, framework_deps); + ClassifyDependency(inherited_target, &classified_deps); } // Data deps. for (const auto& data_dep_pair : target_->data_deps()) - non_linkable_deps->push_back(data_dep_pair.ptr); + classified_deps.non_linkable_deps.push_back(data_dep_pair.ptr); + + return classified_deps; } void NinjaBinaryTargetWriter::ClassifyDependency( const Target* dep, - UniqueVector<OutputFile>* extra_object_files, - UniqueVector<const Target*>* linkable_deps, - UniqueVector<const Target*>* non_linkable_deps, - UniqueVector<const Target*>* framework_deps) const { + ClassifiedDeps* classified_deps) const { // Only the following types of outputs have libraries linked into them: // EXECUTABLE // SHARED_LIBRARY @@ -180,28 +171,28 @@ // set can easily get linked more than once which will cause // multiple definition errors. if (can_link_libs) - AddSourceSetFiles(dep, extra_object_files); + AddSourceSetFiles(dep, &classified_deps->extra_object_files); // Add the source set itself as a non-linkable dependency on the current // target. This will make sure that anything the source set's stamp file // depends on (like data deps) are also built before the current target // can be complete. Otherwise, these will be skipped since this target // will depend only on the source set's object files. - non_linkable_deps->push_back(dep); + classified_deps->non_linkable_deps.push_back(dep); } else if (target_->output_type() == Target::RUST_LIBRARY && dep->IsLinkable()) { // Rust libraries aren't final, but need to have the link lines of all // transitive deps specified. - linkable_deps->push_back(dep); + classified_deps->linkable_deps.push_back(dep); } else if (target_->complete_static_lib() && dep->IsFinal()) { - non_linkable_deps->push_back(dep); + classified_deps->non_linkable_deps.push_back(dep); } else if (can_link_libs && dep->IsLinkable()) { - linkable_deps->push_back(dep); + classified_deps->linkable_deps.push_back(dep); } else if (dep->output_type() == Target::CREATE_BUNDLE && dep->bundle_data().is_framework()) { - framework_deps->push_back(dep); + classified_deps->framework_deps.push_back(dep); } else { - non_linkable_deps->push_back(dep); + classified_deps->non_linkable_deps.push_back(dep); } }
diff --git a/src/gn/ninja_binary_target_writer.h b/src/gn/ninja_binary_target_writer.h index a9d10d5..142ca9e 100644 --- a/src/gn/ninja_binary_target_writer.h +++ b/src/gn/ninja_binary_target_writer.h
@@ -24,6 +24,14 @@ void Run() override; protected: + // Structure used to return the classified deps from |GetDeps| method. + struct ClassifiedDeps { + UniqueVector<OutputFile> extra_object_files; + UniqueVector<const Target*> linkable_deps; + UniqueVector<const Target*> non_linkable_deps; + UniqueVector<const Target*> framework_deps; + }; + // Writes to the output stream a stamp rule for inputs, and // returns the file to be appended to source rules that encodes the // implicit dependencies for the current target. @@ -39,20 +47,14 @@ // Gets all target dependencies and classifies them, as well as accumulates // object files from source sets we need to link. - void GetDeps(UniqueVector<OutputFile>* extra_object_files, - UniqueVector<const Target*>* linkable_deps, - UniqueVector<const Target*>* non_linkable_deps, - UniqueVector<const Target*>* framework_deps) const; + ClassifiedDeps GetClassifiedDeps() const; // Classifies the dependency as linkable or nonlinkable with the current - // target, adding it to the appropriate vector. If the dependency is a source - // set we should link in, the source set's object files will be appended to - // |extra_object_files|. + // target, adding it to the appropriate vector of |classified_deps|. If the + // dependency is a source set we should link in, the source set's object + // files will be appended to |classified_deps.extra_object_files|. void ClassifyDependency(const Target* dep, - UniqueVector<OutputFile>* extra_object_files, - UniqueVector<const Target*>* linkable_deps, - UniqueVector<const Target*>* non_linkable_deps, - UniqueVector<const Target*>* framework_deps) const; + ClassifiedDeps* classified_deps) const; OutputFile WriteStampAndGetDep(const UniqueVector<const SourceFile*>& files, const std::string& stamp_ext) const;
diff --git a/src/gn/ninja_c_binary_target_writer.cc b/src/gn/ninja_c_binary_target_writer.cc index 721b67c..80448a8 100644 --- a/src/gn/ninja_c_binary_target_writer.cc +++ b/src/gn/ninja_c_binary_target_writer.cc
@@ -458,21 +458,16 @@ out_ << ": " << rule_prefix_ << Tool::GetToolTypeForTargetFinalOutput(target_); - UniqueVector<OutputFile> extra_object_files; - UniqueVector<const Target*> linkable_deps; - UniqueVector<const Target*> non_linkable_deps; - UniqueVector<const Target*> framework_deps; - GetDeps(&extra_object_files, &linkable_deps, &non_linkable_deps, - &framework_deps); + ClassifiedDeps classified_deps = GetClassifiedDeps(); // Object files. path_output_.WriteFiles(out_, object_files); - path_output_.WriteFiles(out_, extra_object_files); + path_output_.WriteFiles(out_, classified_deps.extra_object_files); // Dependencies. std::vector<OutputFile> implicit_deps; std::vector<OutputFile> solibs; - for (const Target* cur : linkable_deps) { + for (const Target* cur : classified_deps.linkable_deps) { // All linkable deps should have a link output file. DCHECK(!cur->link_output_file().value().empty()) << "No link output file for " @@ -521,10 +516,8 @@ // always necessary to relink the current target if one of the framework // is regenerated, but it ensure that if one of the framework API changes, // any dependent target will relink it (see crbug.com/1037607). - if (!framework_deps.empty()) { - for (const Target* dep : framework_deps) { - implicit_deps.push_back(dep->dependency_output_file()); - } + for (const Target* dep : classified_deps.framework_deps) { + implicit_deps.push_back(dep->dependency_output_file()); } // The input dependency is only needed if there are no object files, as the @@ -564,7 +557,7 @@ // the sources, there is already an implicit order-only dependency. However, // it's extra work to separate these out and there's no disadvantage to // listing them again. - WriteOrderOnlyDependencies(non_linkable_deps); + WriteOrderOnlyDependencies(classified_deps.non_linkable_deps); // End of the link "build" line. out_ << std::endl;
diff --git a/src/gn/ninja_rust_binary_target_writer.cc b/src/gn/ninja_rust_binary_target_writer.cc index 70b0f1d..59960a1 100644 --- a/src/gn/ninja_rust_binary_target_writer.cc +++ b/src/gn/ninja_rust_binary_target_writer.cc
@@ -118,12 +118,7 @@ WriteCompilerVars(); // Classify our dependencies. - UniqueVector<const Target*> linkable_deps; - UniqueVector<const Target*> non_linkable_deps; - UniqueVector<const Target*> framework_deps; - UniqueVector<OutputFile> extra_obj_files; - GetDeps(&extra_obj_files, &linkable_deps, &non_linkable_deps, - &framework_deps); + ClassifiedDeps classified_deps = GetClassifiedDeps(); // The input dependencies will be an order-only dependency. This will cause // Ninja to make sure the inputs are up to date before compiling this source, @@ -140,23 +135,25 @@ // for ninja dependency tracking. UniqueVector<OutputFile> implicit_deps; AppendSourcesAndInputsToImplicitDeps(&implicit_deps); - implicit_deps.Append(extra_obj_files.begin(), extra_obj_files.end()); + implicit_deps.Append(classified_deps.extra_object_files.begin(), + classified_deps.extra_object_files.end()); std::vector<OutputFile> rustdeps; std::vector<OutputFile> nonrustdeps; - nonrustdeps.insert(nonrustdeps.end(), extra_obj_files.begin(), - extra_obj_files.end()); - for (const auto* framework_dep : framework_deps) { + nonrustdeps.insert(nonrustdeps.end(), + classified_deps.extra_object_files.begin(), + classified_deps.extra_object_files.end()); + for (const auto* framework_dep : classified_deps.framework_deps) { order_only_deps.push_back(framework_dep->dependency_output_file()); } - for (const auto* non_linkable_dep : non_linkable_deps) { + for (const auto* non_linkable_dep : classified_deps.non_linkable_deps) { if (non_linkable_dep->source_types_used().RustSourceUsed() && non_linkable_dep->output_type() != Target::SOURCE_SET) { rustdeps.push_back(non_linkable_dep->dependency_output_file()); } order_only_deps.push_back(non_linkable_dep->dependency_output_file()); } - for (const auto* linkable_dep : linkable_deps) { + for (const auto* linkable_dep : classified_deps.linkable_deps) { if (linkable_dep->source_types_used().RustSourceUsed()) { rustdeps.push_back(linkable_dep->link_output_file()); } else { @@ -192,8 +189,10 @@ implicit_deps.vector(), order_only_deps, tool_->name(), tool_outputs); - std::vector<const Target*> extern_deps(linkable_deps.vector()); - std::copy(non_linkable_deps.begin(), non_linkable_deps.end(), + std::vector<const Target*> extern_deps( + classified_deps.linkable_deps.vector()); + std::copy(classified_deps.non_linkable_deps.begin(), + classified_deps.non_linkable_deps.end(), std::back_inserter(extern_deps)); WriteExterns(extern_deps); WriteRustdeps(transitive_rustlibs, rustdeps, nonrustdeps);