This is the first step of a refactor I'm going to do to separate filling out the target and connecting the dependency tree. I'm hoping this refactor will dramatically simplify the threading code and fix some flakiness. This change is mostly mechanical and replaces the lists of targets and config in the target object with a new pair that consists of a label and the pointer. Currently, they're still filled out at the same time. The new structure also tracks the origin of the thing which simplifies some of the tracking in the describe function. R=scottmg@chromium.org Review URL: https://codereview.chromium.org/48523006 Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src Cr-Mirrored-Commit: 68d1dd345e06e259a086c2ccc186f7f260bbf0d0
diff --git a/tools/gn/BUILD.gn b/tools/gn/BUILD.gn index 3cbb7ff..6195ff3 100644 --- a/tools/gn/BUILD.gn +++ b/tools/gn/BUILD.gn
@@ -74,6 +74,7 @@ "item_tree.h", "label.cc", "label.h", + "label_ptr.h", "location.cc", "location.h", "ninja_binary_target_writer.cc",
diff --git a/tools/gn/command_desc.cc b/tools/gn/command_desc.cc index 14af2d1..415666c 100644 --- a/tools/gn/command_desc.cc +++ b/tools/gn/command_desc.cc
@@ -21,12 +21,6 @@ namespace { -struct CompareTargetLabel { - bool operator()(const Target* a, const Target* b) const { - return a->label() < b->label(); - } -}; - void RecursiveCollectChildDeps(const Target* target, std::set<Label>* result); void RecursiveCollectDeps(const Target* target, std::set<Label>* result) { @@ -38,30 +32,30 @@ } void RecursiveCollectChildDeps(const Target* target, std::set<Label>* result) { - const std::vector<const Target*>& deps = target->deps(); + const LabelTargetVector& deps = target->deps(); for (size_t i = 0; i < deps.size(); i++) - RecursiveCollectDeps(deps[i], result); + RecursiveCollectDeps(deps[i].ptr, result); - const std::vector<const Target*>& datadeps = target->datadeps(); + const LabelTargetVector& datadeps = target->datadeps(); for (size_t i = 0; i < datadeps.size(); i++) - RecursiveCollectDeps(datadeps[i], result); + RecursiveCollectDeps(datadeps[i].ptr, result); } // Prints dependencies of the given target (not the target itself). void RecursivePrintDeps(const Target* target, const Label& default_toolchain, int indent_level) { - std::vector<const Target*> sorted_deps = target->deps(); - const std::vector<const Target*> datadeps = target->datadeps(); - for (size_t i = 0; i < datadeps.size(); i++) - sorted_deps.push_back(datadeps[i]); - std::sort(sorted_deps.begin(), sorted_deps.end(), CompareTargetLabel()); + LabelTargetVector sorted_deps = target->deps(); + const LabelTargetVector& datadeps = target->datadeps(); + sorted_deps.insert(sorted_deps.end(), datadeps.begin(), datadeps.end()); + std::sort(sorted_deps.begin(), sorted_deps.end(), + LabelPtrLabelLess<Target>()); std::string indent(indent_level * 2, ' '); for (size_t i = 0; i < sorted_deps.size(); i++) { OutputString(indent + - sorted_deps[i]->label().GetUserVisibleName(default_toolchain) + "\n"); - RecursivePrintDeps(sorted_deps[i], default_toolchain, indent_level + 1); + sorted_deps[i].label.GetUserVisibleName(default_toolchain) + "\n"); + RecursivePrintDeps(sorted_deps[i].ptr, default_toolchain, indent_level + 1); } } @@ -94,13 +88,13 @@ "(try also \"--all\" and \"--tree\"):\n"); } - const std::vector<const Target*>& target_deps = target->deps(); + const LabelTargetVector& target_deps = target->deps(); for (size_t i = 0; i < target_deps.size(); i++) - deps.push_back(target_deps[i]->label()); + deps.push_back(target_deps[i].label); - const std::vector<const Target*>& target_datadeps = target->datadeps(); + const LabelTargetVector& target_datadeps = target->datadeps(); for (size_t i = 0; i < target_datadeps.size(); i++) - deps.push_back(target_datadeps[i]->label()); + deps.push_back(target_datadeps[i].label); } std::sort(deps.begin(), deps.end()); @@ -141,10 +135,10 @@ OutputString("\nConfigs (in order applying):\n"); Label toolchain_label = target->label().GetToolchainLabel(); - const std::vector<const Config*>& configs = target->configs(); + const LabelConfigVector& configs = target->configs(); for (size_t i = 0; i < configs.size(); i++) { OutputString(" " + - configs[i]->label().GetUserVisibleName(toolchain_label) + "\n"); + configs[i].label.GetUserVisibleName(toolchain_label) + "\n"); } } @@ -158,29 +152,12 @@ OutputString(" " + sources[i].value() + "\n"); } -// Attempts to attribute the gen dependency of the given target to some source -// code and outputs the string to the output stream. -// -// The attribution of the source of the dependencies is stored in the ItemNode -// which is the parallel structure to the target dependency map, so we have -// to jump through a few loops to find everything. -void OutputSourceOfDep(const Target* target, - const Label& dep_label, - std::ostream& out) { - ItemTree& item_tree = target->settings()->build_settings()->item_tree(); - base::AutoLock lock(item_tree.lock()); - - const ItemNode* target_node = target->item_node(); - CHECK(target_node); - ItemNode* dep_node = item_tree.GetExistingNodeLocked(dep_label); - CHECK(dep_node); - - const ItemNode::ItemNodeMap& direct_deps = target_node->direct_dependencies(); - ItemNode::ItemNodeMap::const_iterator found = direct_deps.find(dep_node); - if (found == direct_deps.end()) +// Attribute the origin for attributing from where a target came from. Does +// nothing if the input is null or it does not have a location. +void OutputSourceOfDep(const ParseNode* origin, std::ostream& out) { + if (!origin) return; - - const Location& location = found->second.begin(); + Location location = origin->GetRange().begin(); out << " (Added by " + location.file()->name().value() << ":" << location.line_number() << ")\n"; } @@ -224,7 +201,7 @@ if (config) { // Source of this value is a config. out << " From " << config->label().GetUserVisibleName(false) << "\n"; - OutputSourceOfDep(target, config->label(), out); + OutputSourceOfDep(iter.origin(), out); } else { // Source of this value is the target itself. out << " From " << target->label().GetUserVisibleName(false) << "\n";
diff --git a/tools/gn/command_gyp.cc b/tools/gn/command_gyp.cc index 3c6bef9..0aff1a9 100644 --- a/tools/gn/command_gyp.cc +++ b/tools/gn/command_gyp.cc
@@ -96,11 +96,11 @@ return false; } for (size_t i = 0; i < group.debug->deps().size(); i++) { - if (group.debug->deps()[i]->label() != group.release->deps()[i]->label()) { + if (group.debug->deps()[i].label != group.release->deps()[i].label) { *err = Err(Location(), "The debug and release version of the target \n" + group.debug->label().GetUserVisibleName(true) + "\ndon't agree on the dep\n" + - group.debug->deps()[i]->label().GetUserVisibleName(true)); + group.debug->deps()[i].label.GetUserVisibleName(true)); return false; } }
diff --git a/tools/gn/config_values_extractors.h b/tools/gn/config_values_extractors.h index 4541339..00eb968 100644 --- a/tools/gn/config_values_extractors.h +++ b/tools/gn/config_values_extractors.h
@@ -40,7 +40,15 @@ const ConfigValues& cur() const { if (cur_index_ == -1) return target_->config_values(); - return target_->configs()[cur_index_]->config_values(); + return target_->configs()[cur_index_].ptr->config_values(); + } + + // Returns the origin of who added this config, if any. This will alwsya be + // null for the config values of a target itself. + const ParseNode* origin() const { + if (cur_index_ == -1) + return NULL; + return target_->configs()[cur_index_].origin; } void Next() { @@ -52,7 +60,7 @@ const Config* GetCurrentConfig() const { if (cur_index_ == -1) return NULL; - return target_->configs()[cur_index_]; + return target_->configs()[cur_index_].ptr; } private:
diff --git a/tools/gn/gn.gyp b/tools/gn/gn.gyp index 5dc4c5e..a4f4c34 100644 --- a/tools/gn/gn.gyp +++ b/tools/gn/gn.gyp
@@ -78,6 +78,7 @@ 'item_tree.h', 'label.cc', 'label.h', + 'label_ptr.h', 'location.cc', 'location.h', 'ninja_binary_target_writer.cc',
diff --git a/tools/gn/gyp_binary_target_writer.cc b/tools/gn/gyp_binary_target_writer.cc index bcbe7ee..f668fd3 100644 --- a/tools/gn/gyp_binary_target_writer.cc +++ b/tools/gn/gyp_binary_target_writer.cc
@@ -98,12 +98,12 @@ // and adds them to the given result vector. template<typename T> void FillConfigListValues( - const std::vector<const Config*>& configs, + const LabelConfigVector& configs, const std::vector<T>& (ConfigValues::* getter)() const, std::vector<T>* result) { for (size_t config_i = 0; config_i < configs.size(); config_i++) { const std::vector<T>& values = - (configs[config_i]->config_values().*getter)(); + (configs[config_i].ptr->config_values().*getter)(); for (size_t val_i = 0; val_i < values.size(); val_i++) result->push_back(values[val_i]); } @@ -446,7 +446,7 @@ } void GypBinaryTargetWriter::WriteDeps(const Target* target, int indent) { - const std::vector<const Target*>& deps = target->deps(); + const LabelTargetVector& deps = target->deps(); if (deps.empty()) return; @@ -456,7 +456,7 @@ Indent(indent) << "'dependencies': [\n"; for (size_t i = 0; i < deps.size(); i++) { Indent(indent + kExtraIndent) << "'"; - EscapeStringToStream(out_, helper_.GetFullRefForTarget(deps[i]), + EscapeStringToStream(out_, helper_.GetFullRefForTarget(deps[i].ptr), escape_options); out_ << "',\n"; } @@ -546,7 +546,7 @@ } GypBinaryTargetWriter::Flags GypBinaryTargetWriter::FlagsFromConfigList( - const std::vector<const Config*>& configs) const { + const LabelConfigVector& configs) const { Flags ret; #define EXTRACT(type, name) \
diff --git a/tools/gn/gyp_binary_target_writer.h b/tools/gn/gyp_binary_target_writer.h index 226ffb8..6fee4ad 100644 --- a/tools/gn/gyp_binary_target_writer.h +++ b/tools/gn/gyp_binary_target_writer.h
@@ -10,6 +10,7 @@ #include "base/compiler_specific.h" #include "tools/gn/gyp_target_writer.h" +#include "tools/gn/target.h" #include "tools/gn/toolchain.h" // Writes a portion of a .gyp file for a binary target type (an executable, a @@ -75,7 +76,7 @@ // Fills the given flags structure. Flags FlagsFromTarget(const Target* target) const; - Flags FlagsFromConfigList(const std::vector<const Config*>& configs) const; + Flags FlagsFromConfigList(const LabelConfigVector& configs) const; // All associated targets. TargetGroup group_;
diff --git a/tools/gn/label_ptr.h b/tools/gn/label_ptr.h new file mode 100644 index 0000000..9bc3df2 --- /dev/null +++ b/tools/gn/label_ptr.h
@@ -0,0 +1,80 @@ +// Copyright (c) 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef TOOLS_GN_LABEL_PTR_H_ +#define TOOLS_GN_LABEL_PTR_H_ + +#include <functional> + +class ParseNode; + +// Structure that holds a labeled "thing". This is used for various places +// where we need to store lists of targets or configs. We sometimes populate +// the pointers on another thread from where we compute the labels, so this +// structure lets us save them separately. This also allows us to store the +// location of the thing that added this dependency. +template<typename T> +struct LabelPtrPair { + typedef T DestType; + + LabelPtrPair() : label(), ptr(NULL), origin(NULL) {} + + // This contructor is typically used in unit tests, it extracts the label + // automatically from a given pointer. + explicit LabelPtrPair(const T* p) : label(p->label()), ptr(p), origin(NULL) { + } + + ~LabelPtrPair() {} + + Label label; + const T* ptr; // May be NULL. + const ParseNode* origin; // May be NULL. +}; + +typedef LabelPtrPair<Config> LabelConfigPair; +typedef LabelPtrPair<Target> LabelTargetPair; + +typedef std::vector<LabelConfigPair> LabelConfigVector; +typedef std::vector<LabelTargetPair> LabelTargetVector; + +// Comparison and search functions --------------------------------------------- + +// To do a brute-force search by label: +// std::find_if(vect.begin(), vect.end(), LabelPtrLabelEquals<Config>(label)); +template<typename T> +struct LabelPtrLabelEquals : public std::unary_function<Label, bool> { + explicit LabelPtrLabelEquals(const Label& l) : label(l) {} + + bool operator()(const LabelPtrPair<T>& arg) const { + return arg.label == label; + } + + const Label& label; +}; + +// To do a brute-force search by object pointer: +// std::find_if(vect.begin(), vect.end(), LabelPtrLabelEquals<Config>(config)); +template<typename T> +struct LabelPtrPtrEquals : public std::unary_function<T, bool> { + explicit LabelPtrPtrEquals(const T* p) : ptr(p) {} + + bool operator()(const LabelPtrPair<T>& arg) const { + return arg.ptr == ptr; + } + + const T* ptr; +}; + +// To sort by label: +// std::sort(vect.begin(), vect.end(), LabelPtrLabelLess<Config>()); +template<typename T> +struct LabelPtrLabelLess : public std::binary_function<LabelPtrPair<T>, + LabelPtrPair<T>, + bool> { + bool operator()(const LabelPtrPair<T>& a, const LabelPtrPair<T>& b) const { + return a.label < b.label; + } +}; + +#endif // TOOLS_GN_LABEL_PTR_H_
diff --git a/tools/gn/ninja_binary_target_writer.cc b/tools/gn/ninja_binary_target_writer.cc index 85dd0cd..3a1d620 100644 --- a/tools/gn/ninja_binary_target_writer.cc +++ b/tools/gn/ninja_binary_target_writer.cc
@@ -374,14 +374,14 @@ std::set<OutputFile>* extra_object_files, std::vector<const Target*>* linkable_deps, std::vector<const Target*>* non_linkable_deps) const { - const std::vector<const Target*>& deps = target_->deps(); + const LabelTargetVector& deps = target_->deps(); const std::set<const Target*>& inherited = target_->inherited_libraries(); // Normal deps. for (size_t i = 0; i < deps.size(); i++) { - if (inherited.find(deps[i]) != inherited.end()) + if (inherited.find(deps[i].ptr) != inherited.end()) continue; // Don't add dupes. - ClassifyDependency(deps[i], extra_object_files, + ClassifyDependency(deps[i].ptr, extra_object_files, linkable_deps, non_linkable_deps); } @@ -393,9 +393,9 @@ } // Data deps. - const std::vector<const Target*>& datadeps = target_->datadeps(); + const LabelTargetVector& datadeps = target_->datadeps(); for (size_t i = 0; i < datadeps.size(); i++) - non_linkable_deps->push_back(datadeps[i]); + non_linkable_deps->push_back(datadeps[i].ptr); } void NinjaBinaryTargetWriter::ClassifyDependency(
diff --git a/tools/gn/ninja_binary_target_writer_unittest.cc b/tools/gn/ninja_binary_target_writer_unittest.cc index 0bf4722..a7de4a1 100644 --- a/tools/gn/ninja_binary_target_writer_unittest.cc +++ b/tools/gn/ninja_binary_target_writer_unittest.cc
@@ -50,7 +50,7 @@ // A shared library that depends on the source set. Target shlib_target(setup.settings(), Label(SourceDir("//foo/"), "shlib")); shlib_target.set_output_type(Target::SHARED_LIBRARY); - shlib_target.deps().push_back(&target); + shlib_target.deps().push_back(LabelTargetPair(&target)); shlib_target.OnResolved(); {
diff --git a/tools/gn/ninja_group_target_writer.cc b/tools/gn/ninja_group_target_writer.cc index df95a82..606f916 100644 --- a/tools/gn/ninja_group_target_writer.cc +++ b/tools/gn/ninja_group_target_writer.cc
@@ -24,16 +24,16 @@ << helper_.GetRulePrefix(target_->settings()->toolchain()) << "stamp"; - const std::vector<const Target*>& deps = target_->deps(); + const LabelTargetVector& deps = target_->deps(); for (size_t i = 0; i < deps.size(); i++) { out_ << " "; - path_output_.WriteFile(out_, helper_.GetTargetOutputFile(deps[i])); + path_output_.WriteFile(out_, helper_.GetTargetOutputFile(deps[i].ptr)); } - const std::vector<const Target*>& datadeps = target_->datadeps(); + const LabelTargetVector& datadeps = target_->datadeps(); for (size_t i = 0; i < datadeps.size(); i++) { out_ << " "; - path_output_.WriteFile(out_, helper_.GetTargetOutputFile(datadeps[i])); + path_output_.WriteFile(out_, helper_.GetTargetOutputFile(datadeps[i].ptr)); } out_ << std::endl; }
diff --git a/tools/gn/ninja_target_writer.cc b/tools/gn/ninja_target_writer.cc index a7bd6cc..d73c873 100644 --- a/tools/gn/ninja_target_writer.cc +++ b/tools/gn/ninja_target_writer.cc
@@ -102,12 +102,12 @@ } // Add on any direct deps marked as "hard". - const std::vector<const Target*>& deps = target_->deps(); + const LabelTargetVector& deps = target_->deps(); for (size_t i = 0; i < deps.size(); i++) { - if (deps[i]->hard_dep()) { + if (deps[i].ptr->hard_dep()) { has_files = true; ret << " "; - path_output_.WriteFile(ret, helper_.GetTargetOutputFile(deps[i])); + path_output_.WriteFile(ret, helper_.GetTargetOutputFile(deps[i].ptr)); } }
diff --git a/tools/gn/target.cc b/tools/gn/target.cc index a380156..291d579 100644 --- a/tools/gn/target.cc +++ b/tools/gn/target.cc
@@ -22,12 +22,11 @@ // not be added again. void MergeDirectDependentConfigsFrom(const Target* from_target, ConfigSet* unique_configs, - std::vector<const Config*>* dest) { - const std::vector<const Config*>& direct = - from_target->direct_dependent_configs(); + LabelConfigVector* dest) { + const LabelConfigVector& direct = from_target->direct_dependent_configs(); for (size_t i = 0; i < direct.size(); i++) { - if (unique_configs->find(direct[i]) == unique_configs->end()) { - unique_configs->insert(direct[i]); + if (unique_configs->find(direct[i].ptr) == unique_configs->end()) { + unique_configs->insert(direct[i].ptr); dest->push_back(direct[i]); } } @@ -38,20 +37,19 @@ // the dest target given in *all_dest. void MergeAllDependentConfigsFrom(const Target* from_target, ConfigSet* unique_configs, - std::vector<const Config*>* dest, - std::vector<const Config*>* all_dest) { - const std::vector<const Config*>& all = - from_target->all_dependent_configs(); + LabelConfigVector* dest, + LabelConfigVector* all_dest) { + const LabelConfigVector& all = from_target->all_dependent_configs(); for (size_t i = 0; i < all.size(); i++) { // Always add it to all_dependent_configs_ since it might not be in that // list even if we've seen it applied to this target before. This may // introduce some duplicates in all_dependent_configs_, but those will // we removed when they're actually applied to a target. all_dest->push_back(all[i]); - if (unique_configs->find(all[i]) == unique_configs->end()) { + if (unique_configs->find(all[i].ptr) == unique_configs->end()) { // One we haven't seen yet, also apply it to ourselves. dest->push_back(all[i]); - unique_configs->insert(all[i]); + unique_configs->insert(all[i].ptr); } } } @@ -110,7 +108,7 @@ // flags, etc. that it specifies itself are applied to us. size_t original_deps_size = deps_.size(); for (size_t i = 0; i < original_deps_size; i++) { - const Target* dep = deps_[i]; + const Target* dep = deps_[i].ptr; if (dep->output_type_ == GROUP) { deps_.insert(deps_.begin() + i + 1, dep->deps_.begin(), dep->deps_.end()); i += dep->deps_.size(); @@ -120,21 +118,21 @@ // Only add each config once. First remember the target's configs. ConfigSet unique_configs; for (size_t i = 0; i < configs_.size(); i++) - unique_configs.insert(configs_[i]); + unique_configs.insert(configs_[i].ptr); // Copy our own dependent configs to the list of configs applying to us. for (size_t i = 0; i < all_dependent_configs_.size(); i++) { - const Config* cur = all_dependent_configs_[i]; - if (unique_configs.find(cur) == unique_configs.end()) { - unique_configs.insert(cur); - configs_.push_back(cur); + if (unique_configs.find(all_dependent_configs_[i].ptr) == + unique_configs.end()) { + unique_configs.insert(all_dependent_configs_[i].ptr); + configs_.push_back(all_dependent_configs_[i]); } } for (size_t i = 0; i < direct_dependent_configs_.size(); i++) { - const Config* cur = direct_dependent_configs_[i]; - if (unique_configs.find(cur) == unique_configs.end()) { - unique_configs.insert(cur); - configs_.push_back(cur); + if (unique_configs.find(direct_dependent_configs_[i].ptr) == + unique_configs.end()) { + unique_configs.insert(direct_dependent_configs_[i].ptr); + configs_.push_back(direct_dependent_configs_[i]); } } @@ -181,7 +179,7 @@ void Target::PullDependentTargetInfo(std::set<const Config*>* unique_configs) { // Gather info from our dependents we need. for (size_t dep_i = 0; dep_i < deps_.size(); dep_i++) { - const Target* dep = deps_[dep_i]; + const Target* dep = deps_[dep_i].ptr; MergeAllDependentConfigsFrom(dep, unique_configs, &configs_, &all_dependent_configs_); MergeDirectDependentConfigsFrom(dep, unique_configs, &configs_); @@ -212,15 +210,16 @@ // Forward direct dependent configs if requested. for (size_t dep = 0; dep < forward_dependent_configs_.size(); dep++) { - const Target* from_target = forward_dependent_configs_[dep]; + const Target* from_target = forward_dependent_configs_[dep].ptr; // The forward_dependent_configs_ must be in the deps already, so we // don't need to bother copying to our configs, only forwarding. - DCHECK(std::find(deps_.begin(), deps_.end(), from_target) != + DCHECK(std::find_if(deps_.begin(), deps_.end(), + LabelPtrPtrEquals<Target>(from_target)) != deps_.end()); - const std::vector<const Config*>& direct = - from_target->direct_dependent_configs(); - for (size_t i = 0; i < direct.size(); i++) - direct_dependent_configs_.push_back(direct[i]); + direct_dependent_configs_.insert( + direct_dependent_configs_.end(), + from_target->direct_dependent_configs().begin(), + from_target->direct_dependent_configs().end()); } }
diff --git a/tools/gn/target.h b/tools/gn/target.h index 8dfc4cc..d5c1af3 100644 --- a/tools/gn/target.h +++ b/tools/gn/target.h
@@ -16,6 +16,7 @@ #include "base/synchronization/lock.h" #include "tools/gn/config_values.h" #include "tools/gn/item.h" +#include "tools/gn/label_ptr.h" #include "tools/gn/ordered_set.h" #include "tools/gn/script_values.h" #include "tools/gn/source_file.h" @@ -84,42 +85,42 @@ void set_hard_dep(bool hd) { hard_dep_ = hd; } // Linked dependencies. - const std::vector<const Target*>& deps() const { return deps_; } - std::vector<const Target*>& deps() { return deps_; } + const LabelTargetVector& deps() const { return deps_; } + LabelTargetVector& deps() { return deps_; } // Non-linked dependencies. - const std::vector<const Target*>& datadeps() const { return datadeps_; } - std::vector<const Target*>& datadeps() { return datadeps_; } + const LabelTargetVector& datadeps() const { return datadeps_; } + LabelTargetVector& datadeps() { return datadeps_; } // List of configs that this class inherits settings from. - const std::vector<const Config*>& configs() const { return configs_; } - std::vector<const Config*>& configs() { return configs_; } + const LabelConfigVector& configs() const { return configs_; } + LabelConfigVector& configs() { return configs_; } // List of configs that all dependencies (direct and indirect) of this // target get. These configs are not added to this target. Note that due // to the way this is computed, there may be duplicates in this list. - const std::vector<const Config*>& all_dependent_configs() const { + const LabelConfigVector& all_dependent_configs() const { return all_dependent_configs_; } - std::vector<const Config*>& all_dependent_configs() { + LabelConfigVector& all_dependent_configs() { return all_dependent_configs_; } // List of configs that targets depending directly on this one get. These // configs are not added to this target. - const std::vector<const Config*>& direct_dependent_configs() const { + const LabelConfigVector& direct_dependent_configs() const { return direct_dependent_configs_; } - std::vector<const Config*>& direct_dependent_configs() { + LabelConfigVector& direct_dependent_configs() { return direct_dependent_configs_; } // A list of a subset of deps where we'll re-export direct_dependent_configs // as direct_dependent_configs of this target. - const std::vector<const Target*>& forward_dependent_configs() const { + const LabelTargetVector& forward_dependent_configs() const { return forward_dependent_configs_; } - std::vector<const Target*>& forward_dependent_configs() { + LabelTargetVector& forward_dependent_configs() { return forward_dependent_configs_; } @@ -169,13 +170,13 @@ // so we also need the group in the list so we find these things. But you // shouldn't need to look inside the deps of the group since those will // already be added. - std::vector<const Target*> deps_; - std::vector<const Target*> datadeps_; + LabelTargetVector deps_; + LabelTargetVector datadeps_; - std::vector<const Config*> configs_; - std::vector<const Config*> all_dependent_configs_; - std::vector<const Config*> direct_dependent_configs_; - std::vector<const Target*> forward_dependent_configs_; + LabelConfigVector configs_; + LabelConfigVector all_dependent_configs_; + LabelConfigVector direct_dependent_configs_; + LabelTargetVector forward_dependent_configs_; bool external_;
diff --git a/tools/gn/target_generator.cc b/tools/gn/target_generator.cc index d5bcfe2..3bb51eb 100644 --- a/tools/gn/target_generator.cc +++ b/tools/gn/target_generator.cc
@@ -144,14 +144,14 @@ } void TargetGenerator::FillConfigs() { - FillGenericConfigs(variables::kConfigs, &Target::configs); + FillGenericConfigs(variables::kConfigs, &target_->configs()); } void TargetGenerator::FillDependentConfigs() { FillGenericConfigs(variables::kAllDependentConfigs, - &Target::all_dependent_configs); + &target_->all_dependent_configs()); FillGenericConfigs(variables::kDirectDependentConfigs, - &Target::direct_dependent_configs); + &target_->direct_dependent_configs()); } void TargetGenerator::FillData() { @@ -169,8 +169,8 @@ } void TargetGenerator::FillDependencies() { - FillGenericDeps(variables::kDeps, &Target::deps); - FillGenericDeps(variables::kDatadeps, &Target::datadeps); + FillGenericDeps(variables::kDeps, &target_->deps()); + FillGenericDeps(variables::kDatadeps, &target_->datadeps()); // This is a list of dependent targets to have their configs fowarded, so // it goes here rather than in FillConfigs. @@ -240,53 +240,41 @@ GetBuildSettings(), function_token_.range(), tc_node, err_); } -void TargetGenerator::FillGenericConfigs( - const char* var_name, - std::vector<const Config*>& (Target::*accessor)()) { +void TargetGenerator::FillGenericConfigs(const char* var_name, + LabelConfigVector* dest) { const Value* value = scope_->GetValue(var_name, true); if (!value) return; - - std::vector<Label> labels; if (!ExtractListOfLabels(*value, scope_->GetSourceDir(), - ToolchainLabelForScope(scope_), &labels, err_)) + ToolchainLabelForScope(scope_), dest, err_)) return; - std::vector<const Config*> dest_configs; - dest_configs.resize(labels.size()); - for (size_t i = 0; i < labels.size(); i++) { - dest_configs[i] = Config::GetConfig( - scope_->settings(), - value->list_value()[i].origin()->GetRange(), - labels[i], target_, err_); + for (size_t i = 0; i < dest->size(); i++) { + LabelConfigPair& cur = (*dest)[i]; + cur.ptr = Config::GetConfig(scope_->settings(), + value->list_value()[i].origin()->GetRange(), + cur.label, target_, err_); if (err_->has_error()) return; } - (target_->*accessor)().swap(dest_configs); } -void TargetGenerator::FillGenericDeps( - const char* var_name, - std::vector<const Target*>& (Target::*accessor)()) { +void TargetGenerator::FillGenericDeps(const char* var_name, + LabelTargetVector* dest) { const Value* value = scope_->GetValue(var_name, true); if (!value) return; - - std::vector<Label> labels; if (!ExtractListOfLabels(*value, scope_->GetSourceDir(), - ToolchainLabelForScope(scope_), &labels, err_)) + ToolchainLabelForScope(scope_), dest, err_)) return; - std::vector<const Target*> dest_deps; - dest_deps.resize(labels.size()); - for (size_t i = 0; i < labels.size(); i++) { - dest_deps[i] = GetBuildSettings()->target_manager().GetTarget( - labels[i], value->list_value()[i].origin()->GetRange(), target_, err_); + for (size_t i = 0; i < dest->size(); i++) { + LabelTargetPair& cur = (*dest)[i]; + cur.ptr = GetBuildSettings()->target_manager().GetTarget( + cur.label, value->list_value()[i].origin()->GetRange(), target_, err_); if (err_->has_error()) return; } - - (target_->*accessor)().swap(dest_deps); } void TargetGenerator::FillForwardDependentConfigs() { @@ -295,36 +283,31 @@ if (!value) return; - std::vector<Label> labels; + LabelTargetVector& dest = target_->forward_dependent_configs(); if (!ExtractListOfLabels(*value, scope_->GetSourceDir(), - ToolchainLabelForScope(scope_), &labels, err_)) + ToolchainLabelForScope(scope_), &dest, err_)) return; - const std::vector<const Target*>& deps = target_->deps(); - // We currently assume that the list is very small and do a brute-force // search in the deps for the labeled target. This could be optimized. + const LabelTargetVector& deps = target_->deps(); std::vector<const Target*> forward_from_list; - for (size_t label_index = 0; label_index < labels.size(); label_index++) { - const Target* forward_from = NULL; + for (size_t dest_index = 0; dest_index < dest.size(); dest_index++) { + LabelTargetPair& cur_dest = dest[dest_index]; for (size_t dep_index = 0; dep_index < deps.size(); dep_index++) { - if (deps[dep_index]->label() == labels[label_index]) { - forward_from = deps[dep_index]; + if (deps[dep_index].label == cur_dest.label) { + cur_dest.ptr = deps[dep_index].ptr; break; } } - if (!forward_from) { - *err_ = Err(value->list_value()[label_index], + if (!cur_dest.ptr) { + *err_ = Err(cur_dest.origin, "Can't forward from this target.", "forward_dependent_configs_from must contain a list of labels that\n" "must all appear in the deps of the same target."); return; } - - forward_from_list.push_back(forward_from); } - - target_->forward_dependent_configs().swap(forward_from_list); }
diff --git a/tools/gn/target_generator.h b/tools/gn/target_generator.h index 074061d..583e57f 100644 --- a/tools/gn/target_generator.h +++ b/tools/gn/target_generator.h
@@ -72,10 +72,8 @@ // Reads configs/deps from the given var name, and uses the given setting on // the target to save them. - void FillGenericConfigs(const char* var_name, - std::vector<const Config*>& (Target::*accessor)()); - void FillGenericDeps(const char* var_name, - std::vector<const Target*>& (Target::*accessor)()); + void FillGenericConfigs(const char* var_name, LabelConfigVector* dest); + void FillGenericDeps(const char* var_name, LabelTargetVector* dest); void FillForwardDependentConfigs();
diff --git a/tools/gn/target_unittest.cc b/tools/gn/target_unittest.cc index d68b65c..ea1700f 100644 --- a/tools/gn/target_unittest.cc +++ b/tools/gn/target_unittest.cc
@@ -39,8 +39,8 @@ // Make a group for both x and y. Target g(&settings_, Label(SourceDir("//group/"), "g")); g.set_output_type(Target::GROUP); - g.deps().push_back(&x); - g.deps().push_back(&y); + g.deps().push_back(LabelTargetPair(&x)); + g.deps().push_back(LabelTargetPair(&y)); // Random placeholder target so we can see the group's deps get inserted at // the right place. @@ -49,17 +49,17 @@ // Make a target depending on the group and "b". OnResolved will expand. Target a(&settings_, Label(SourceDir("//app/"), "a")); a.set_output_type(Target::EXECUTABLE); - a.deps().push_back(&g); - a.deps().push_back(&b); + a.deps().push_back(LabelTargetPair(&g)); + a.deps().push_back(LabelTargetPair(&b)); a.OnResolved(); // The group's deps should be inserted after the group itself in the deps // list, so we should get "g, x, y, b" ASSERT_EQ(4u, a.deps().size()); - EXPECT_EQ(&g, a.deps()[0]); - EXPECT_EQ(&x, a.deps()[1]); - EXPECT_EQ(&y, a.deps()[2]); - EXPECT_EQ(&b, a.deps()[3]); + EXPECT_EQ(&g, a.deps()[0].ptr); + EXPECT_EQ(&x, a.deps()[1].ptr); + EXPECT_EQ(&y, a.deps()[2].ptr); + EXPECT_EQ(&b, a.deps()[3].ptr); } // Tests that lib[_dir]s are inherited across deps boundaries for static @@ -89,7 +89,7 @@ shared.set_output_type(Target::SHARED_LIBRARY); shared.config_values().libs().push_back(second_lib); shared.config_values().lib_dirs().push_back(second_libdir); - shared.deps().push_back(&z); + shared.deps().push_back(LabelTargetPair(&z)); shared.OnResolved(); ASSERT_EQ(2u, shared.all_libs().size()); @@ -102,7 +102,7 @@ // Executable target shouldn't get either by depending on shared. Target exec(&settings_, Label(SourceDir("//foo/"), "exec")); exec.set_output_type(Target::EXECUTABLE); - exec.deps().push_back(&shared); + exec.deps().push_back(LabelTargetPair(&shared)); exec.OnResolved(); EXPECT_EQ(0u, exec.all_libs().size()); EXPECT_EQ(0u, exec.all_lib_dirs().size()); @@ -118,20 +118,20 @@ b.set_output_type(Target::STATIC_LIBRARY); Target c(&settings_, Label(SourceDir("//foo/"), "c")); c.set_output_type(Target::STATIC_LIBRARY); - a.deps().push_back(&b); - b.deps().push_back(&c); + a.deps().push_back(LabelTargetPair(&b)); + b.deps().push_back(LabelTargetPair(&c)); // Normal non-inherited config. Config config(Label(SourceDir("//foo/"), "config")); - c.configs().push_back(&config); + c.configs().push_back(LabelConfigPair(&config)); // All dependent config. Config all(Label(SourceDir("//foo/"), "all")); - c.all_dependent_configs().push_back(&all); + c.all_dependent_configs().push_back(LabelConfigPair(&all)); // Direct dependent config. Config direct(Label(SourceDir("//foo/"), "direct")); - c.direct_dependent_configs().push_back(&direct); + c.direct_dependent_configs().push_back(LabelConfigPair(&direct)); c.OnResolved(); b.OnResolved(); @@ -139,32 +139,32 @@ // B should have gotten both dependent configs from C. ASSERT_EQ(2u, b.configs().size()); - EXPECT_EQ(&all, b.configs()[0]); - EXPECT_EQ(&direct, b.configs()[1]); + EXPECT_EQ(&all, b.configs()[0].ptr); + EXPECT_EQ(&direct, b.configs()[1].ptr); ASSERT_EQ(1u, b.all_dependent_configs().size()); - EXPECT_EQ(&all, b.all_dependent_configs()[0]); + EXPECT_EQ(&all, b.all_dependent_configs()[0].ptr); // A should have just gotten the "all" dependent config from C. ASSERT_EQ(1u, a.configs().size()); - EXPECT_EQ(&all, a.configs()[0]); - EXPECT_EQ(&all, a.all_dependent_configs()[0]); + EXPECT_EQ(&all, a.configs()[0].ptr); + EXPECT_EQ(&all, a.all_dependent_configs()[0].ptr); // Making an an alternate A and B with B forwarding the direct dependents. Target a_fwd(&settings_, Label(SourceDir("//foo/"), "a_fwd")); a_fwd.set_output_type(Target::EXECUTABLE); Target b_fwd(&settings_, Label(SourceDir("//foo/"), "b_fwd")); b_fwd.set_output_type(Target::STATIC_LIBRARY); - a_fwd.deps().push_back(&b_fwd); - b_fwd.deps().push_back(&c); - b_fwd.forward_dependent_configs().push_back(&c); + a_fwd.deps().push_back(LabelTargetPair(&b_fwd)); + b_fwd.deps().push_back(LabelTargetPair(&c)); + b_fwd.forward_dependent_configs().push_back(LabelTargetPair(&c)); b_fwd.OnResolved(); a_fwd.OnResolved(); // A_fwd should now have both configs. ASSERT_EQ(2u, a_fwd.configs().size()); - EXPECT_EQ(&all, a_fwd.configs()[0]); - EXPECT_EQ(&direct, a_fwd.configs()[1]); + EXPECT_EQ(&all, a_fwd.configs()[0].ptr); + EXPECT_EQ(&direct, a_fwd.configs()[1].ptr); ASSERT_EQ(1u, a_fwd.all_dependent_configs().size()); - EXPECT_EQ(&all, a_fwd.all_dependent_configs()[0]); + EXPECT_EQ(&all, a_fwd.all_dependent_configs()[0].ptr); }
diff --git a/tools/gn/value_extractors.cc b/tools/gn/value_extractors.cc index c2ca285..5d11bc4 100644 --- a/tools/gn/value_extractors.cc +++ b/tools/gn/value_extractors.cc
@@ -55,15 +55,17 @@ const SourceDir& current_dir; }; -struct LabelResolver { +// Fills the label part of a LabelPtrPair, leaving the pointer null. +template<typename T> struct LabelResolver { LabelResolver(const SourceDir& current_dir_in, const Label& current_toolchain_in) : current_dir(current_dir_in), current_toolchain(current_toolchain_in) {} - bool operator()(const Value& v, Label* out, Err* err) const { + bool operator()(const Value& v, LabelPtrPair<T>* out, Err* err) const { if (!v.VerifyTypeIs(Value::STRING, err)) return false; - *out = Label::Resolve(current_dir, current_toolchain, v, err); + out->label = Label::Resolve(current_dir, current_toolchain, v, err); + out->origin = v.origin(); return !err->has_error(); } const SourceDir& current_dir; @@ -108,8 +110,19 @@ bool ExtractListOfLabels(const Value& value, const SourceDir& current_dir, const Label& current_toolchain, - std::vector<Label>* dest, + LabelConfigVector* dest, Err* err) { return ListValueExtractor(value, dest, err, - LabelResolver(current_dir, current_toolchain)); + LabelResolver<Config>(current_dir, + current_toolchain)); +} + +bool ExtractListOfLabels(const Value& value, + const SourceDir& current_dir, + const Label& current_toolchain, + LabelTargetVector* dest, + Err* err) { + return ListValueExtractor(value, dest, err, + LabelResolver<Target>(current_dir, + current_toolchain)); }
diff --git a/tools/gn/value_extractors.h b/tools/gn/value_extractors.h index 12fff00..ff1611d 100644 --- a/tools/gn/value_extractors.h +++ b/tools/gn/value_extractors.h
@@ -8,6 +8,7 @@ #include <string> #include <vector> +#include "tools/gn/target.h" #include "tools/gn/value.h" class BuildSettings; @@ -51,10 +52,17 @@ std::vector<SourceDir>* dest, Err* err); +// Extracts the list of labels and their origins to the given vector. Only the +// labels are filled in, the ptr for each pair in the vector will be null. bool ExtractListOfLabels(const Value& value, const SourceDir& current_dir, const Label& current_toolchain, - std::vector<Label>* dest, + LabelConfigVector* dest, + Err* err); +bool ExtractListOfLabels(const Value& value, + const SourceDir& current_dir, + const Label& current_toolchain, + LabelTargetVector* dest, Err* err); #endif // TOOLS_GN_VALUE_EXTRACTORS_H_