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(); }