Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1189)

Unified Diff: tools/gn/scope.cc

Issue 2148993002: Allow multiple set_default calls in GN. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: . Created 4 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « tools/gn/scope.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: tools/gn/scope.cc
diff --git a/tools/gn/scope.cc b/tools/gn/scope.cc
index 93ce966e4f3e8abfcb0bc20ce9e7a374ed56f9a5..821b940b435be53843fe91f1d58f88a6a4d0b280 100644
--- a/tools/gn/scope.cc
+++ b/tools/gn/scope.cc
@@ -298,18 +298,27 @@ bool Scope::NonRecursiveMergeTo(Scope* dest,
}
if (!options.clobber_existing) {
- if (dest->GetTargetDefaults(current_name)) {
- // TODO(brettw) it would be nice to know the origin of a
- // set_target_defaults so we can give locations for the colliding target
- // defaults.
- std::string desc_string(desc_for_err);
- *err = Err(node_for_err, "Target defaults collision.",
- "This " + desc_string + " contains target defaults for\n"
- "\"" + current_name + "\" which would clobber one for the\n"
- "same target type in your current scope. It's unfortunate that I'm "
- "too stupid\nto tell you the location of where the target defaults "
- "were set. Usually\nthis happens in the BUILDCONFIG.gn file.");
- return false;
+ const Scope* dest_defaults = dest->GetTargetDefaults(current_name);
+ if (dest_defaults) {
+ if (RecordMapValuesEqual(pair.second->values_,
+ dest_defaults->values_)) {
+ // Values of the two defaults are equivalent, just ignore the
+ // collision.
+ continue;
+ } else {
+ // TODO(brettw) it would be nice to know the origin of a
+ // set_target_defaults so we can give locations for the colliding
+ // target defaults.
+ std::string desc_string(desc_for_err);
+ *err = Err(node_for_err, "Target defaults collision.",
+ "This " + desc_string + " contains target defaults for\n"
+ "\"" + current_name + "\" which would clobber one for the\n"
+ "same target type in your current scope. It's unfortunate that "
+ "I'm too stupid\nto tell you the location of where the target "
+ "defaults were set. Usually\nthis happens in the BUILDCONFIG.gn "
+ "file or in a related .gni file.\n");
+ return false;
+ }
}
}
@@ -404,14 +413,7 @@ std::unique_ptr<Scope> Scope::MakeClosure() const {
}
Scope* Scope::MakeTargetDefaults(const std::string& target_type) {
- if (GetTargetDefaults(target_type))
- return nullptr;
-
std::unique_ptr<Scope>& dest = target_defaults_[target_type];
- if (dest) {
- NOTREACHED(); // Already set.
- return dest.get();
- }
dest = base::WrapUnique(new Scope(settings_));
return dest.get();
}
@@ -514,3 +516,17 @@ void Scope::RemoveProvider(ProgrammaticProvider* p) {
DCHECK(programmatic_providers_.find(p) != programmatic_providers_.end());
programmatic_providers_.erase(p);
}
+
+// static
+bool Scope::RecordMapValuesEqual(const RecordMap& a, const RecordMap& b) {
+ if (a.size() != b.size())
+ return false;
+ for (const auto& pair : a) {
+ const auto& found_b = b.find(pair.first);
+ if (found_b == b.end())
+ return false; // Item in 'a' but not 'b'.
+ if (pair.second.value != found_b->second.value)
+ return false; // Values for variable in 'a' and 'b' are different.
+ }
+ return true;
+}
« no previous file with comments | « tools/gn/scope.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698