Throw errors for nested targets in GN.
People sometimes nest targets or configs, usually with the assumption that
this limits the visibility of a config to within a target. But this nesting
provides no visibility restrictions over declaring it outside of a block.
For clarity, force certain types of blocks to be non-nested.
Adds missing ordering documentation to the ldflags variable.
Review URL: https://codereview.chromium.org/1314773005
Cr-Original-Commit-Position: refs/heads/master@{#348186}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: a88eb18a5cb883cbde7a1e055958642a8dafb4ac
diff --git a/tools/gn/function_template.cc b/tools/gn/function_template.cc
index 1f1da82..290eb78 100644
--- a/tools/gn/function_template.cc
+++ b/tools/gn/function_template.cc
@@ -163,6 +163,14 @@
const std::vector<Value>& args,
BlockNode* block,
Err* err) {
+ // Of course you can have configs and targets in a template. But here, we're
+ // not actually executing the block, only declaring it. Marking the template
+ // declaration as non-nestable means that you can't put it inside a target,
+ // for example.
+ NonNestableBlock non_nestable(scope, function, "template");
+ if (!non_nestable.Enter(err))
+ return Value();
+
// TODO(brettw) determine if the function is built-in and throw an error if
// it is.
if (args.size() != 1) {
diff --git a/tools/gn/function_toolchain.cc b/tools/gn/function_toolchain.cc
index 2fe5f53..4e1fe2d 100644
--- a/tools/gn/function_toolchain.cc
+++ b/tools/gn/function_toolchain.cc
@@ -301,6 +301,10 @@
const std::vector<Value>& args,
BlockNode* block,
Err* err) {
+ NonNestableBlock non_nestable(scope, function, "toolchain");
+ if (!non_nestable.Enter(err))
+ return Value();
+
if (!EnsureNotProcessingImport(function, scope, err) ||
!EnsureNotProcessingBuildConfig(function, scope, err))
return Value();
diff --git a/tools/gn/functions.cc b/tools/gn/functions.cc
index 0d86093..71d05aa 100644
--- a/tools/gn/functions.cc
+++ b/tools/gn/functions.cc
@@ -131,6 +131,42 @@
toolchain_label.name());
}
+// static
+const int NonNestableBlock::kKey = 0;
+
+NonNestableBlock::NonNestableBlock(
+ Scope* scope,
+ const FunctionCallNode* function,
+ const char* type_description)
+ : scope_(scope),
+ function_(function),
+ type_description_(type_description),
+ key_added_(false) {
+}
+
+NonNestableBlock::~NonNestableBlock() {
+ if (key_added_)
+ scope_->SetProperty(&kKey, nullptr);
+}
+
+bool NonNestableBlock::Enter(Err* err) {
+ void* scope_value = scope_->GetProperty(&kKey, nullptr);
+ if (scope_value) {
+ // Existing block.
+ const NonNestableBlock* existing =
+ reinterpret_cast<const NonNestableBlock*>(scope_value);
+ *err = Err(function_, "Can't nest these things.",
+ std::string("You are trying to nest a ") + type_description_ +
+ " inside a " + existing->type_description_ + ".");
+ err->AppendSubErr(Err(existing->function_, "The enclosing block."));
+ return false;
+ }
+
+ scope_->SetProperty(&kKey, this);
+ key_added_ = true;
+ return true;
+}
+
namespace functions {
// assert ----------------------------------------------------------------------
@@ -244,6 +280,10 @@
const std::vector<Value>& args,
Scope* scope,
Err* err) {
+ NonNestableBlock non_nestable(scope, function, "config");
+ if (!non_nestable.Enter(err))
+ return Value();
+
if (!EnsureSingleStringArg(function, args, err) ||
!EnsureNotProcessingImport(function, scope, err))
return Value();
@@ -308,6 +348,10 @@
const std::vector<Value>& args,
BlockNode* block,
Err* err) {
+ NonNestableBlock non_nestable(scope, function, "declare_args");
+ if (!non_nestable.Enter(err))
+ return Value();
+
Scope block_scope(scope);
block->Execute(&block_scope, err);
if (err->has_error())
diff --git a/tools/gn/functions.h b/tools/gn/functions.h
index 6015375..f9c867d 100644
--- a/tools/gn/functions.h
+++ b/tools/gn/functions.h
@@ -430,4 +430,40 @@
const FunctionCallNode* function,
const std::string& name);
+// Some tyesp of blocks can't be nested inside other ones. For such cases,
+// instantiate this object upon entering the block and Enter() will fail if
+// there is already another non-nestable block on the stack.
+class NonNestableBlock {
+ public:
+ enum Type {
+ CONFIG,
+ DECLARE_ARGS,
+ TARGET,
+ TEMPLATE,
+ TOOLCHAIN,
+ };
+
+ // type_description is a string that will be used in error messages
+ // describing the type of the block, for example, "template" or "config".
+ NonNestableBlock(Scope* scope,
+ const FunctionCallNode* function,
+ const char* type_description);
+ ~NonNestableBlock();
+
+ bool Enter(Err* err);
+
+ private:
+ // Used as a void* key for the Scope to track our property. The actual value
+ // is never used.
+ static const int kKey;
+
+ Scope* scope_;
+ const FunctionCallNode* function_;
+ const char* type_description_;
+
+ // Set to true when the key is added to the scope so we don't try to
+ // delete nonexistant keys which will cause assertions.
+ bool key_added_;
+};
+
#endif // TOOLS_GN_FUNCTIONS_H_
diff --git a/tools/gn/functions_target.cc b/tools/gn/functions_target.cc
index 655c32a..45e6859 100644
--- a/tools/gn/functions_target.cc
+++ b/tools/gn/functions_target.cc
@@ -31,6 +31,10 @@
const std::vector<Value>& args,
BlockNode* block,
Err* err) {
+ NonNestableBlock non_nestable(scope, function, "target");
+ if (!non_nestable.Enter(err))
+ return Value();
+
if (!EnsureNotProcessingImport(function, scope, err) ||
!EnsureNotProcessingBuildConfig(function, scope, err))
return Value();
diff --git a/tools/gn/variables.cc b/tools/gn/variables.cc
index 0f3ede7..83b2957 100644
--- a/tools/gn/variables.cc
+++ b/tools/gn/variables.cc
@@ -767,7 +767,8 @@
" ldflags are NOT pushed to dependents, so applying ldflags to source\n"
" sets or static libraries will be a no-op. If you want to apply ldflags\n"
" to dependent targets, put them in a config and set it in the\n"
- " all_dependent_configs or public_configs.\n";
+ " all_dependent_configs or public_configs.\n"
+ COMMON_ORDERING_HELP;
#define COMMON_LIB_INHERITANCE_HELP \
"\n" \
