Commit a88eb18a authored by brettw's avatar brettw Committed by Commit bot

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-Commit-Position: refs/heads/master@{#348186}
parent 87fae95a
...@@ -163,6 +163,14 @@ Value RunTemplate(Scope* scope, ...@@ -163,6 +163,14 @@ Value RunTemplate(Scope* scope,
const std::vector<Value>& args, const std::vector<Value>& args,
BlockNode* block, BlockNode* block,
Err* err) { 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 // TODO(brettw) determine if the function is built-in and throw an error if
// it is. // it is.
if (args.size() != 1) { if (args.size() != 1) {
......
...@@ -301,6 +301,10 @@ Value RunToolchain(Scope* scope, ...@@ -301,6 +301,10 @@ Value RunToolchain(Scope* scope,
const std::vector<Value>& args, const std::vector<Value>& args,
BlockNode* block, BlockNode* block,
Err* err) { Err* err) {
NonNestableBlock non_nestable(scope, function, "toolchain");
if (!non_nestable.Enter(err))
return Value();
if (!EnsureNotProcessingImport(function, scope, err) || if (!EnsureNotProcessingImport(function, scope, err) ||
!EnsureNotProcessingBuildConfig(function, scope, err)) !EnsureNotProcessingBuildConfig(function, scope, err))
return Value(); return Value();
......
...@@ -131,6 +131,42 @@ Label MakeLabelForScope(const Scope* scope, ...@@ -131,6 +131,42 @@ Label MakeLabelForScope(const Scope* scope,
toolchain_label.name()); 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 { namespace functions {
// assert ---------------------------------------------------------------------- // assert ----------------------------------------------------------------------
...@@ -244,6 +280,10 @@ Value RunConfig(const FunctionCallNode* function, ...@@ -244,6 +280,10 @@ Value RunConfig(const FunctionCallNode* function,
const std::vector<Value>& args, const std::vector<Value>& args,
Scope* scope, Scope* scope,
Err* err) { Err* err) {
NonNestableBlock non_nestable(scope, function, "config");
if (!non_nestable.Enter(err))
return Value();
if (!EnsureSingleStringArg(function, args, err) || if (!EnsureSingleStringArg(function, args, err) ||
!EnsureNotProcessingImport(function, scope, err)) !EnsureNotProcessingImport(function, scope, err))
return Value(); return Value();
...@@ -308,6 +348,10 @@ Value RunDeclareArgs(Scope* scope, ...@@ -308,6 +348,10 @@ Value RunDeclareArgs(Scope* scope,
const std::vector<Value>& args, const std::vector<Value>& args,
BlockNode* block, BlockNode* block,
Err* err) { Err* err) {
NonNestableBlock non_nestable(scope, function, "declare_args");
if (!non_nestable.Enter(err))
return Value();
Scope block_scope(scope); Scope block_scope(scope);
block->Execute(&block_scope, err); block->Execute(&block_scope, err);
if (err->has_error()) if (err->has_error())
......
...@@ -430,4 +430,40 @@ Label MakeLabelForScope(const Scope* scope, ...@@ -430,4 +430,40 @@ Label MakeLabelForScope(const Scope* scope,
const FunctionCallNode* function, const FunctionCallNode* function,
const std::string& name); 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_ #endif // TOOLS_GN_FUNCTIONS_H_
...@@ -31,6 +31,10 @@ Value ExecuteGenericTarget(const char* target_type, ...@@ -31,6 +31,10 @@ Value ExecuteGenericTarget(const char* target_type,
const std::vector<Value>& args, const std::vector<Value>& args,
BlockNode* block, BlockNode* block,
Err* err) { Err* err) {
NonNestableBlock non_nestable(scope, function, "target");
if (!non_nestable.Enter(err))
return Value();
if (!EnsureNotProcessingImport(function, scope, err) || if (!EnsureNotProcessingImport(function, scope, err) ||
!EnsureNotProcessingBuildConfig(function, scope, err)) !EnsureNotProcessingBuildConfig(function, scope, err))
return Value(); return Value();
......
...@@ -767,7 +767,8 @@ const char kLdflags_Help[] = ...@@ -767,7 +767,8 @@ const char kLdflags_Help[] =
" ldflags are NOT pushed to dependents, so applying ldflags to source\n" " 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" " 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" " 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 \ #define COMMON_LIB_INHERITANCE_HELP \
"\n" \ "\n" \
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment