Update GN toolchain_args to be a variable.
This makes toolchain_args on a toolchain definition a variable instead of a function call.
The function call is kept for backwards compatibility (for now) and it just sets the variable.
forward_variables_from now accepts a first parameter of any type that evaluates to a scope, which allows things like "invoker.toolchain_args" to be used there.
Updates "gn format" to format scope assignments without extra indents.
BUG=634446
Review-Url: https://codereview.chromium.org/2219083002
Cr-Original-Commit-Position: refs/heads/master@{#410123}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 22227896dc68aa88ba1836cd53fd261ba21bef88
diff --git a/tools/gn/args.cc b/tools/gn/args.cc
index 581c8ac..9aaafb0 100644
--- a/tools/gn/args.cc
+++ b/tools/gn/args.cc
@@ -35,7 +35,7 @@
" toolchain_args section of a toolchain definition. The use-case for\n"
" this is that a toolchain may be building code for a different\n"
" platform, and that it may want to always specify Posix, for example.\n"
- " See \"gn help toolchain_args\" for more.\n"
+ " See \"gn help toolchain\" for more.\n"
"\n"
" If you specify an override for a build argument that never appears in\n"
" a \"declare_args\" call, a nonfatal error will be displayed.\n"
diff --git a/tools/gn/command_format.cc b/tools/gn/command_format.cc
index 7b8f2f3..13d8cc8 100644
--- a/tools/gn/command_format.cc
+++ b/tools/gn/command_format.cc
@@ -507,17 +507,28 @@
bool is_assignment = binop->op().value() == "=" ||
binop->op().value() == "+=" ||
binop->op().value() == "-=";
- // A sort of funny special case for the long lists that are common in .gn
- // files, don't indent them + 4, even though they're just continuations when
- // they're simple lists like "x = [ a, b, c, ... ]"
- const ListNode* right_as_list = binop->right()->AsList();
- int indent_column =
- (is_assignment &&
- (!right_as_list || (!right_as_list->prefer_multiline() &&
- !ListWillBeMultiline(right_as_list->contents(),
- right_as_list->End()))))
- ? margin() + kIndentSize * 2
- : start_column;
+
+ int indent_column = start_column;
+ if (is_assignment) {
+ // Default to a double-indent for wrapped assignments.
+ indent_column = margin() + kIndentSize * 2;
+
+ // A special case for the long lists and scope assignments that are
+ // common in .gn files, don't indent them + 4, even though they're just
+ // continuations when they're simple lists like "x = [ a, b, c, ... ]" or
+ // scopes like "x = { a = 1 b = 2 }". Put back to "normal" indenting.
+ const ListNode* right_as_list = binop->right()->AsList();
+ if (right_as_list) {
+ if (right_as_list->prefer_multiline() ||
+ ListWillBeMultiline(right_as_list->contents(),
+ right_as_list->End()))
+ indent_column = start_column;
+ } else {
+ const BlockNode* right_as_block = binop->right()->AsBlock();
+ if (right_as_block)
+ indent_column = start_column;
+ }
+ }
if (stack_.back().continuation_requires_indent)
indent_column += kIndentSize * 2;
diff --git a/tools/gn/command_format_unittest.cc b/tools/gn/command_format_unittest.cc
index 7c79d41..d753b33 100644
--- a/tools/gn/command_format_unittest.cc
+++ b/tools/gn/command_format_unittest.cc
@@ -104,3 +104,4 @@
FORMAT_TEST(064)
FORMAT_TEST(065)
FORMAT_TEST(066)
+FORMAT_TEST(067)
diff --git a/tools/gn/format_test_data/067.gn b/tools/gn/format_test_data/067.gn
new file mode 100644
index 0000000..b3d5eaa
--- /dev/null
+++ b/tools/gn/format_test_data/067.gn
@@ -0,0 +1,8 @@
+# Scope wrapping.
+
+myscope = {
+}
+myscope = { a = 1 }
+myscope = { a = 1 b = 2}
+# Comment
+SomeFunction({a=1}, "foo")
diff --git a/tools/gn/format_test_data/067.golden b/tools/gn/format_test_data/067.golden
new file mode 100644
index 0000000..9ce2fa7
--- /dev/null
+++ b/tools/gn/format_test_data/067.golden
@@ -0,0 +1,17 @@
+# Scope wrapping.
+
+myscope = {
+}
+myscope = {
+ a = 1
+}
+myscope = {
+ a = 1
+ b = 2
+}
+
+# Comment
+SomeFunction({
+ a = 1
+ },
+ "foo")
diff --git a/tools/gn/function_forward_variables_from.cc b/tools/gn/function_forward_variables_from.cc
index 8c7a558..0445f8b 100644
--- a/tools/gn/function_forward_variables_from.cc
+++ b/tools/gn/function_forward_variables_from.cc
@@ -172,23 +172,27 @@
return Value();
}
- // Extract the scope identifier. This assumes the first parameter is an
- // identifier. It is difficult to write code where this is not the case, and
- // this saves an expensive scope copy. If necessary, this could be expanded
- // to execute the ParseNode and get the value out if it's not an identifer.
+ Value* value = nullptr; // Value to use, may point to result_value.
+ Value result_value; // Storage for the "evaluate" case.
const IdentifierNode* identifier = args_vector[0]->AsIdentifier();
- if (!identifier) {
- *err = Err(args_vector[0].get(), "Expected an identifier for the scope.");
- return Value();
+ if (identifier) {
+ // Optimize the common case where the input scope is an identifier. This
+ // prevents a copy of a potentially large Scope object.
+ value = scope->GetMutableValue(identifier->value().value(),
+ Scope::SEARCH_NESTED, true);
+ if (!value) {
+ *err = Err(identifier, "Undefined identifier.");
+ return Value();
+ }
+ } else {
+ // Non-optimized case, just evaluate the argument.
+ result_value = args_vector[0]->Execute(scope, err);
+ if (err->has_error())
+ return Value();
+ value = &result_value;
}
// Extract the source scope.
- Value* value = scope->GetMutableValue(
- identifier->value().value(), Scope::SEARCH_NESTED, true);
- if (!value) {
- *err = Err(identifier, "Undefined identifier.");
- return Value();
- }
if (!value->VerifyTypeIs(Value::SCOPE, err))
return Value();
Scope* source = value->scope_value();
diff --git a/tools/gn/function_forward_variables_from_unittest.cc b/tools/gn/function_forward_variables_from_unittest.cc
index 2e6f5a2..84d8b6e 100644
--- a/tools/gn/function_forward_variables_from_unittest.cc
+++ b/tools/gn/function_forward_variables_from_unittest.cc
@@ -50,6 +50,28 @@
}
}
+TEST(FunctionForwardVariablesFrom, LiteralList) {
+ Scheduler scheduler;
+ TestWithScope setup;
+
+ // Forwards all variables from a literal scope into another scope definition.
+ TestParseInput input(
+ "a = {\n"
+ " forward_variables_from({x = 1 y = 2}, \"*\")\n"
+ " z = 3\n"
+ "}\n"
+ "print(\"${a.x} ${a.y} ${a.z}\")\n");
+
+ ASSERT_FALSE(input.has_error());
+
+ Err err;
+ input.parsed()->Execute(setup.scope(), &err);
+ ASSERT_FALSE(err.has_error()) << err.message();
+
+ EXPECT_EQ("1 2 3\n", setup.print_output());
+ setup.print_output().clear();
+}
+
TEST(FunctionForwardVariablesFrom, ListWithExclusion) {
Scheduler scheduler;
TestWithScope setup;
@@ -94,7 +116,7 @@
Err err;
invalid_source.parsed()->Execute(setup.scope(), &err);
EXPECT_TRUE(err.has_error());
- EXPECT_EQ("Expected an identifier for the scope.", err.message());
+ EXPECT_EQ("This is not a scope.", err.message());
// Type check the list. We need to use a new template name each time since
// all of these invocations are executing in sequence in the same scope.
diff --git a/tools/gn/function_toolchain.cc b/tools/gn/function_toolchain.cc
index faa190e..852d25a 100644
--- a/tools/gn/function_toolchain.cc
+++ b/tools/gn/function_toolchain.cc
@@ -314,10 +314,26 @@
" The tool() function call specifies the commands commands to run for\n"
" a given step. See \"gn help tool\".\n"
"\n"
- " toolchain_args()\n"
- " List of arguments to pass to the toolchain when invoking this\n"
- " toolchain. This applies only to non-default toolchains. See\n"
- " \"gn help toolchain_args\" for more.\n"
+ " toolchain_args\n"
+ " Overrides for build arguments to pass to the toolchain when invoking\n"
+ " it. This is a variable of type \"scope\" where the variable names\n"
+ " correspond to varibles in declare_args() blocks.\n"
+ "\n"
+ " When you specify a target using an alternate toolchain, the master\n"
+ " build configuration file is re-interpreted in the context of that\n"
+ " toolchain (see \"gn help toolchain\"). The toolchain_args allows you\n"
+ " to control the arguments passed into this alternate invocation of\n"
+ " the build.\n"
+ "\n"
+ " Any default system arguments or arguments passed in via \"gn args\"\n"
+ " will also be passed to the alternate invocation unless explicitly\n"
+ " overridden by toolchain_args.\n"
+ "\n"
+ " The toolchain_args will be ignored when the toolchain being defined\n"
+ " is the default. In this case, it's expected you want the default\n"
+ " argument values.\n"
+ "\n"
+ " See also \"gn help buildargs\" for an overview of these arguments.\n"
"\n"
" deps\n"
" Dependencies of this toolchain. These dependencies will be resolved\n"
@@ -350,18 +366,19 @@
" by the toolchain label).\n"
" 2. Re-runs the master build configuration file, applying the\n"
" arguments specified by the toolchain_args section of the toolchain\n"
- " definition (see \"gn help toolchain_args\").\n"
+ " definition.\n"
" 3. Loads the destination build file in the context of the\n"
" configuration file in the previous step.\n"
"\n"
- "Example:\n"
+ "Example\n"
+ "\n"
" toolchain(\"plugin_toolchain\") {\n"
" tool(\"cc\") {\n"
" command = \"gcc {{source}}\"\n"
" ...\n"
" }\n"
"\n"
- " toolchain_args() {\n"
+ " toolchain_args = {\n"
" is_plugin = true\n"
" is_32bit = true\n"
" is_64bit = false\n"
@@ -412,6 +429,17 @@
return Value();
}
+ // Read toolchain args (if any).
+ const Value* toolchain_args = block_scope.GetValue("toolchain_args", true);
+ if (toolchain_args) {
+ if (!toolchain_args->VerifyTypeIs(Value::SCOPE, err))
+ return Value();
+
+ Scope::KeyValueMap values;
+ toolchain_args->scope_value()->GetCurrentScopeValues(&values);
+ toolchain->args() = values;
+ }
+
if (!block_scope.CheckForUnusedVars(err))
return Value();
@@ -1015,79 +1043,45 @@
return Value();
}
-// toolchain_args --------------------------------------------------------------
-
extern const char kToolchainArgs[] = "toolchain_args";
extern const char kToolchainArgs_HelpShort[] =
"toolchain_args: Set build arguments for toolchain build setup.";
extern const char kToolchainArgs_Help[] =
"toolchain_args: Set build arguments for toolchain build setup.\n"
"\n"
- " Used inside a toolchain definition to pass arguments to an alternate\n"
- " toolchain's invocation of the build.\n"
+ " DEPRECATED. Instead use:\n"
+ " toolchain_args = { ... }\n"
"\n"
- " When you specify a target using an alternate toolchain, the master\n"
- " build configuration file is re-interpreted in the context of that\n"
- " toolchain (see \"gn help toolchain\"). The toolchain_args function\n"
- " allows you to control the arguments passed into this alternate\n"
- " invocation of the build.\n"
- "\n"
- " Any default system arguments or arguments passed in on the command-\n"
- " line will also be passed to the alternate invocation unless explicitly\n"
- " overridden by toolchain_args.\n"
- "\n"
- " The toolchain_args will be ignored when the toolchain being defined\n"
- " is the default. In this case, it's expected you want the default\n"
- " argument values.\n"
- "\n"
- " See also \"gn help buildargs\" for an overview of these arguments.\n"
- "\n"
- "Example:\n"
- " toolchain(\"my_weird_toolchain\") {\n"
- " ...\n"
- " toolchain_args() {\n"
- " # Override the system values for a generic Posix system.\n"
- " is_win = false\n"
- " is_posix = true\n"
- "\n"
- " # Pass this new value for specific setup for my toolchain.\n"
- " is_my_weird_system = true\n"
- " }\n"
- " }\n";
+ " See \"gn help toolchain\" for documentation.\n";
Value RunToolchainArgs(Scope* scope,
const FunctionCallNode* function,
const std::vector<Value>& args,
BlockNode* block,
Err* err) {
- // Find the toolchain definition we're executing inside of. The toolchain
- // function will set a property pointing to it that we'll pick up.
- Toolchain* toolchain = reinterpret_cast<Toolchain*>(
- scope->GetProperty(&kToolchainPropertyKey, nullptr));
- if (!toolchain) {
- *err = Err(function->function(),
- "toolchain_args() called outside of toolchain().",
- "The toolchain_args() function can only be used inside a "
- "toolchain() definition.");
- return Value();
- }
-
+ // This is a backwards-compatible shim that converts the old form of:
+ // toolchain_args() {
+ // foo = bar
+ // }
+ // to the new form:
+ // toolchain_args = {
+ // foo = bar
+ // }
+ // It will be deleted when all users of toolchain_args as a function are
+ // deleted.
if (!args.empty()) {
*err = Err(function->function(), "This function takes no arguments.");
return Value();
}
- // This function makes a new scope with various variable sets on it, which
- // we then save on the toolchain to use when re-invoking the build.
- Scope block_scope(scope);
- block->Execute(&block_scope, err);
+ std::unique_ptr<Scope> block_scope(new Scope(scope));
+ block->Execute(block_scope.get(), err);
if (err->has_error())
return Value();
- Scope::KeyValueMap values;
- block_scope.GetCurrentScopeValues(&values);
- toolchain->args() = values;
-
+ block_scope->DetachFromContaining();
+ scope->SetValue("toolchain_args", Value(function, std::move(block_scope)),
+ function);
return Value();
}
