summaryrefslogtreecommitdiffstats
path: root/tools/gn
diff options
context:
space:
mode:
Diffstat (limited to 'tools/gn')
-rw-r--r--tools/gn/function_set_defaults.cc2
-rw-r--r--tools/gn/functions.cc10
-rw-r--r--tools/gn/import_manager.cc5
-rw-r--r--tools/gn/loader.cc8
-rw-r--r--tools/gn/parse_tree.cc2
-rw-r--r--tools/gn/parse_tree_unittest.cc29
-rw-r--r--tools/gn/scope.cc50
-rw-r--r--tools/gn/scope.h53
-rw-r--r--tools/gn/scope_unittest.cc74
9 files changed, 205 insertions, 28 deletions
diff --git a/tools/gn/function_set_defaults.cc b/tools/gn/function_set_defaults.cc
index ac61fe9..7851113 100644
--- a/tools/gn/function_set_defaults.cc
+++ b/tools/gn/function_set_defaults.cc
@@ -84,7 +84,7 @@ Value RunSetDefaults(Scope* scope,
// Now copy the values set on the scope we made into the free-floating one
// (with no containing scope) used to hold the target defaults.
Scope* dest = scope->MakeTargetDefaults(target_type);
- block_scope.NonRecursiveMergeTo(dest, false, function,
+ block_scope.NonRecursiveMergeTo(dest, Scope::MergeOptions(), function,
"<SHOULD NOT FAIL>", err);
return Value();
}
diff --git a/tools/gn/functions.cc b/tools/gn/functions.cc
index b2787a3..97c2592 100644
--- a/tools/gn/functions.cc
+++ b/tools/gn/functions.cc
@@ -61,8 +61,10 @@ bool FillTargetBlockScope(const Scope* scope,
// the block in.
const Scope* default_scope = scope->GetTargetDefaults(target_type);
if (default_scope) {
- if (!default_scope->NonRecursiveMergeTo(block_scope, false, function,
- "target defaults", err))
+ Scope::MergeOptions merge_options;
+ merge_options.skip_private_vars = true;
+ if (!default_scope->NonRecursiveMergeTo(block_scope, merge_options,
+ function, "target defaults", err))
return false;
}
@@ -446,6 +448,10 @@ const char kImport_Help[] =
" different value), a runtime error will be thrown. Therefore, it's good\n"
" practice to minimize the stuff that an imported file defines.\n"
"\n"
+ " Variables and templates beginning with an underscore '_' are\n"
+ " considered private and will not be imported. Imported files can use\n"
+ " such variables for internal computation without affecting other files.\n"
+ "\n"
"Examples:\n"
"\n"
" import(\"//build/rules/idl_compilation_rule.gni\")\n"
diff --git a/tools/gn/import_manager.cc b/tools/gn/import_manager.cc
index 4bb4b63..842577c 100644
--- a/tools/gn/import_manager.cc
+++ b/tools/gn/import_manager.cc
@@ -86,6 +86,9 @@ bool ImportManager::DoImport(const SourceFile& file,
}
}
- return imported_scope->NonRecursiveMergeTo(scope, false, node_for_err,
+ Scope::MergeOptions options;
+ options.skip_private_vars = true;
+ options.mark_used = true; // Don't require all imported values be used.
+ return imported_scope->NonRecursiveMergeTo(scope, options, node_for_err,
"import", err);
}
diff --git a/tools/gn/loader.cc b/tools/gn/loader.cc
index 4cca2ce..32879ea 100644
--- a/tools/gn/loader.cc
+++ b/tools/gn/loader.cc
@@ -245,6 +245,9 @@ void LoaderImpl::BackgroundLoadFile(const Settings* settings,
if (err.has_error())
g_scheduler->FailWithError(err);
+ if (!our_scope.CheckForUnusedVars(&err))
+ g_scheduler->FailWithError(err);
+
// Pass all of the items that were defined off to the builder.
for (size_t i = 0; i < collected_items.size(); i++)
settings->build_settings()->ItemDefined(collected_items[i]->Pass());
@@ -285,6 +288,11 @@ void LoaderImpl::BackgroundLoadBuildConfig(
Err err;
root_block->ExecuteBlockInScope(base_config, &err);
+ // Clear all private variables left in the scope. We want the root build
+ // config to be like a .gni file in that variables beginning with an
+ // underscore aren't exported.
+ base_config->RemovePrivateIdentifiers();
+
trace.Done();
if (err.has_error())
diff --git a/tools/gn/parse_tree.cc b/tools/gn/parse_tree.cc
index 4015fd1..00fca54 100644
--- a/tools/gn/parse_tree.cc
+++ b/tools/gn/parse_tree.cc
@@ -217,7 +217,7 @@ Value BlockNode::Execute(Scope* containing_scope, Err* err) const {
return Value();
// Check for unused vars in the scope.
- //our_scope.CheckForUnusedVars(err);
+ our_scope.CheckForUnusedVars(err);
return ret;
}
return ExecuteBlockInScope(containing_scope, err);
diff --git a/tools/gn/parse_tree_unittest.cc b/tools/gn/parse_tree_unittest.cc
index 4f46232..fc06c32 100644
--- a/tools/gn/parse_tree_unittest.cc
+++ b/tools/gn/parse_tree_unittest.cc
@@ -48,3 +48,32 @@ TEST(ParseTree, Accessor) {
ASSERT_EQ(Value::INTEGER, result.type());
EXPECT_EQ(kBValue, result.int_value());
}
+
+TEST(ParseTree, BlockUnusedVars) {
+ TestWithScope setup;
+
+ // Printing both values should be OK.
+ TestParseInput input_all_used(
+ "{\n"
+ " a = 12\n"
+ " b = 13\n"
+ " print(\"$a $b\")\n"
+ "}");
+ EXPECT_FALSE(input_all_used.has_error());
+
+ Err err;
+ input_all_used.parsed()->Execute(setup.scope(), &err);
+ EXPECT_FALSE(err.has_error());
+
+ // Skipping one should throw an unused var error.
+ TestParseInput input_unused(
+ "{\n"
+ " a = 12\n"
+ " b = 13\n"
+ " print(\"$a\")\n"
+ "}");
+ EXPECT_FALSE(input_unused.has_error());
+
+ input_unused.parsed()->Execute(setup.scope(), &err);
+ EXPECT_TRUE(err.has_error());
+}
diff --git a/tools/gn/scope.cc b/tools/gn/scope.cc
index ebaf0ef..4e049eb 100644
--- a/tools/gn/scope.cc
+++ b/tools/gn/scope.cc
@@ -16,6 +16,13 @@ namespace {
const unsigned kProcessingBuildConfigFlag = 1;
const unsigned kProcessingImportFlag = 2;
+// Returns true if this variable name should be considered private. Private
+// values start with an underscore, and are not imported from "gni" files
+// when processing an import.
+bool IsPrivateVar(const base::StringPiece& name) {
+ return name.empty() || name[0] == '_';
+}
+
} // namespace
Scope::Scope(const Settings* settings)
@@ -129,6 +136,21 @@ void Scope::RemoveIdentifier(const base::StringPiece& ident) {
values_.erase(found);
}
+void Scope::RemovePrivateIdentifiers() {
+ // Do it in two phases to avoid mutating while iterating. Our hash map is
+ // currently backed by several different vendor-specific implementations and
+ // I'm not sure if all of them support mutating while iterating. Since this
+ // is not perf-critical, do the safe thing.
+ std::vector<base::StringPiece> to_remove;
+ for (RecordMap::const_iterator i = values_.begin(); i != values_.end(); ++i) {
+ if (IsPrivateVar(i->first))
+ to_remove.push_back(i->first);
+ }
+
+ for (size_t i = 0; i < to_remove.size(); i++)
+ values_.erase(to_remove[i]);
+}
+
bool Scope::AddTemplate(const std::string& name, const Template* templ) {
if (GetTemplate(name))
return false;
@@ -201,14 +223,17 @@ void Scope::GetCurrentScopeValues(KeyValueMap* output) const {
}
bool Scope::NonRecursiveMergeTo(Scope* dest,
- bool clobber_existing,
+ const MergeOptions& options,
const ParseNode* node_for_err,
const char* desc_for_err,
Err* err) const {
// Values.
for (RecordMap::const_iterator i = values_.begin(); i != values_.end(); ++i) {
+ if (options.skip_private_vars && IsPrivateVar(i->first))
+ continue; // Skip this private var.
+
const Value& new_value = i->second.value;
- if (!clobber_existing) {
+ if (!options.clobber_existing) {
const Value* existing_value = dest->GetValue(i->first);
if (existing_value && new_value != *existing_value) {
// Value present in both the source and the dest.
@@ -225,12 +250,15 @@ bool Scope::NonRecursiveMergeTo(Scope* dest,
}
}
dest->values_[i->first] = i->second;
+
+ if (options.mark_used)
+ dest->MarkUsed(i->first);
}
// Target defaults are owning pointers.
for (NamedScopeMap::const_iterator i = target_defaults_.begin();
i != target_defaults_.end(); ++i) {
- if (!clobber_existing) {
+ if (!options.clobber_existing) {
if (dest->GetTargetDefaults(i->first)) {
// TODO(brettw) it would be nice to know the origin of a
// set_target_defaults so we can give locations for the colliding target
@@ -251,13 +279,13 @@ bool Scope::NonRecursiveMergeTo(Scope* dest,
if (*dest_scope)
delete *dest_scope;
*dest_scope = new Scope(settings_);
- i->second->NonRecursiveMergeTo(*dest_scope, clobber_existing, node_for_err,
+ i->second->NonRecursiveMergeTo(*dest_scope, options, node_for_err,
"<SHOULDN'T HAPPEN>", err);
}
// Sources assignment filter.
if (sources_assignment_filter_) {
- if (!clobber_existing) {
+ if (!options.clobber_existing) {
if (dest->GetSourcesAssignmentFilter()) {
// Sources assignment filter present in both the source and the dest.
std::string desc_string(desc_for_err);
@@ -274,7 +302,10 @@ bool Scope::NonRecursiveMergeTo(Scope* dest,
// Templates.
for (TemplateMap::const_iterator i = templates_.begin();
i != templates_.end(); ++i) {
- if (!clobber_existing) {
+ if (options.skip_private_vars && IsPrivateVar(i->first))
+ continue; // Skip this private template.
+
+ if (!options.clobber_existing) {
const Template* existing_template = dest->GetTemplate(i->first);
if (existing_template) {
// Rule present in both the source and the dest.
@@ -314,9 +345,14 @@ scoped_ptr<Scope> Scope::MakeClosure() const {
result.reset(new Scope(settings_));
}
+ // Want to clobber since we've flattened some nested scopes, and our parent
+ // scope may have a duplicate value set.
+ MergeOptions options;
+ options.clobber_existing = true;
+
// Add in our variables and we're done.
Err err;
- NonRecursiveMergeTo(result.get(), true, NULL, "<SHOULDN'T HAPPEN>", &err);
+ NonRecursiveMergeTo(result.get(), options, NULL, "<SHOULDN'T HAPPEN>", &err);
DCHECK(!err.has_error());
return result.Pass();
}
diff --git a/tools/gn/scope.h b/tools/gn/scope.h
index 53b3252..035d45b 100644
--- a/tools/gn/scope.h
+++ b/tools/gn/scope.h
@@ -65,6 +65,37 @@ class Scope {
Scope* scope_;
};
+ // Options for configuring scope merges.
+ struct MergeOptions {
+ // Defaults to all false, which are the things least likely to cause errors.
+ MergeOptions()
+ : clobber_existing(false),
+ skip_private_vars(false),
+ mark_used(false) {
+ }
+
+ // When set, all existing avlues in the destination scope will be
+ // overwritten.
+ //
+ // When false, it will be an error to merge a variable into another scope
+ // where a variable with the same name is already set. The exception is
+ // if both of the variables have the same value (which happens if you
+ // somehow multiply import the same file, for example). This case will be
+ // ignored since there is nothing getting lost.
+ bool clobber_existing;
+
+ // When true, private variables (names beginning with an underscore) will
+ // be copied to the destination scope. When false, private values will be
+ // skipped.
+ bool skip_private_vars;
+
+ // When set, values copied to the destination scope will be marked as used
+ // so won't trigger an unused variable warning. You want this when doing an
+ // import, for example, or files that don't need a variable from the .gni
+ // file will throw an error.
+ bool mark_used;
+ };
+
// Creates an empty toplevel scope.
Scope(const Settings* settings);
@@ -138,6 +169,10 @@ class Scope {
// scope. This does not search recursive scopes. Does nothing if not found.
void RemoveIdentifier(const base::StringPiece& ident);
+ // Removes from this scope all identifiers and templates that are considered
+ // private.
+ void RemovePrivateIdentifiers();
+
// Templates associated with this scope. A template can only be set once, so
// AddTemplate will fail and return false if a rule with that name already
// exists. GetTemplate returns NULL if the rule doesn't exist, and it will
@@ -169,30 +204,22 @@ class Scope {
// copied, neither will the reference to the containing scope (this is why
// it's "non-recursive").
//
- // If clobber_existing is true, any existing values will be overwritten. In
- // this mode, this function will never fail.
- //
- // If clobber_existing is false, it will be an error to merge a variable into
- // a scope that already has something with that name in scope (meaning in
- // that scope or in any of its containing scopes). If this happens, the error
- // will be set and the function will return false.
- //
// This is used in different contexts. When generating the error, the given
// parse node will be blamed, and the given desc will be used to describe
// the operation that doesn't support doing this. For example, desc_for_err
// would be "import" when doing an import, and the error string would say
// something like "The import contains...".
bool NonRecursiveMergeTo(Scope* dest,
- bool clobber_existing,
+ const MergeOptions& options,
const ParseNode* node_for_err,
const char* desc_for_err,
Err* err) const;
// Constructs a scope that is a copy of the current one. Nested scopes will
- // be collapsed until we reach a const containing scope. The resulting
- // closure will reference the const containing scope as its containing scope
- // (since we assume the const scope won't change, we don't have to copy its
- // values).
+ // be collapsed until we reach a const containing scope. Private values will
+ // be included. The resulting closure will reference the const containing
+ // scope as its containing scope (since we assume the const scope won't
+ // change, we don't have to copy its values).
scoped_ptr<Scope> MakeClosure() const;
// Makes an empty scope with the given name. Returns NULL if the name is
diff --git a/tools/gn/scope_unittest.cc b/tools/gn/scope_unittest.cc
index 5585e50..53d4689 100644
--- a/tools/gn/scope_unittest.cc
+++ b/tools/gn/scope_unittest.cc
@@ -36,6 +36,8 @@ TEST(Scope, NonRecursiveMergeTo) {
Value old_value(&assignment, "hello");
setup.scope()->SetValue("v", old_value, &assignment);
+ base::StringPiece private_var_name("_private");
+ setup.scope()->SetValue(private_var_name, old_value, &assignment);
// Detect collisions of values' values.
{
@@ -45,7 +47,8 @@ TEST(Scope, NonRecursiveMergeTo) {
Err err;
EXPECT_FALSE(setup.scope()->NonRecursiveMergeTo(
- &new_scope, false, &assignment, "error", &err));
+ &new_scope, Scope::MergeOptions(),
+ &assignment, "error", &err));
EXPECT_TRUE(err.has_error());
}
@@ -56,8 +59,10 @@ TEST(Scope, NonRecursiveMergeTo) {
new_scope.SetValue("v", new_value, &assignment);
Err err;
+ Scope::MergeOptions options;
+ options.clobber_existing = true;
EXPECT_TRUE(setup.scope()->NonRecursiveMergeTo(
- &new_scope, true, &assignment, "error", &err));
+ &new_scope, options, &assignment, "error", &err));
EXPECT_FALSE(err.has_error());
const Value* found_value = new_scope.GetValue("v");
@@ -72,8 +77,61 @@ TEST(Scope, NonRecursiveMergeTo) {
new_scope.SetValue("v", new_value, &assignment);
Err err;
+ Scope::MergeOptions options;
+ options.clobber_existing = true;
EXPECT_TRUE(setup.scope()->NonRecursiveMergeTo(
- &new_scope, false, &assignment, "error", &err));
+ &new_scope, options, &assignment, "error", &err));
+ EXPECT_FALSE(err.has_error());
+ }
+
+ // Copy private values.
+ {
+ Scope new_scope(setup.settings());
+
+ Err err;
+ EXPECT_TRUE(setup.scope()->NonRecursiveMergeTo(
+ &new_scope, Scope::MergeOptions(), &assignment, "error", &err));
+ EXPECT_FALSE(err.has_error());
+ EXPECT_TRUE(new_scope.GetValue(private_var_name));
+ }
+
+ // Skip private values.
+ {
+ Scope new_scope(setup.settings());
+
+ Err err;
+ Scope::MergeOptions options;
+ options.skip_private_vars = true;
+ EXPECT_TRUE(setup.scope()->NonRecursiveMergeTo(
+ &new_scope, options, &assignment, "error", &err));
+ EXPECT_FALSE(err.has_error());
+ EXPECT_FALSE(new_scope.GetValue(private_var_name));
+ }
+
+ // Don't mark used.
+ {
+ Scope new_scope(setup.settings());
+
+ Err err;
+ Scope::MergeOptions options;
+ EXPECT_TRUE(setup.scope()->NonRecursiveMergeTo(
+ &new_scope, options, &assignment, "error", &err));
+ EXPECT_FALSE(err.has_error());
+ EXPECT_FALSE(new_scope.CheckForUnusedVars(&err));
+ EXPECT_TRUE(err.has_error());
+ }
+
+ // Mark used.
+ {
+ Scope new_scope(setup.settings());
+
+ Err err;
+ Scope::MergeOptions options;
+ options.mark_used = true;
+ EXPECT_TRUE(setup.scope()->NonRecursiveMergeTo(
+ &new_scope, options, &assignment, "error", &err));
+ EXPECT_FALSE(err.has_error());
+ EXPECT_TRUE(new_scope.CheckForUnusedVars(&err));
EXPECT_FALSE(err.has_error());
}
}
@@ -167,3 +225,13 @@ TEST(Scope, GetMutableValue) {
ASSERT_TRUE(mutable2_result);
EXPECT_TRUE(*mutable2_result == value);
}
+
+TEST(Scope, RemovePrivateIdentifiers) {
+ TestWithScope setup;
+ setup.scope()->SetValue("a", Value(NULL, true), NULL);
+ setup.scope()->SetValue("_b", Value(NULL, true), NULL);
+
+ setup.scope()->RemovePrivateIdentifiers();
+ EXPECT_TRUE(setup.scope()->GetValue("a"));
+ EXPECT_FALSE(setup.scope()->GetValue("_b"));
+}
generated by cgit v1.1 at 2025-03-19 10:11:12 +0000