Change GN to disallow reading args defined in the same declare_args() call.

tools/gn/functions.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "tools/gn/functions.h"
 
 #include <stddef.h>
 #include <iostream>
 #include <utility>
 
@@ skipping 27 matching lines @@
     return true;
 
   *err = Err(block, "Unexpected '{'.",
       "This function call doesn't take a {} block following it, and you\n"
       "can't have a {} block that's not connected to something like an if\n"
       "statement or a target declaration.");
   err->AppendRange(function->function().range());
   return false;
 }
 
+// This key is set as a scope property on the scope of a declare_args() block,
+// in order to prevent reading a variable defined earlier in the same call
+// (see `gn help declare_args` for more).
+const void *kInDeclareArgsKey = nullptr;
+
 }  // namespace
 
+
+bool EnsureNotReadingFromSameDeclareArgs(const ParseNode* node,
+                                         const Scope* cur_scope,
+                                         const Scope* val_scope,
+                                         Err* err) {
+  // If the value didn't come from a scope at all, we're safe.
+  if (!val_scope)
+    return true;
+
+  const Scope* val_args_scope = nullptr;
+  val_scope->GetProperty(&kInDeclareArgsKey, &val_args_scope);
+
+  const Scope* cur_args_scope = nullptr;
+  cur_scope->GetProperty(&kInDeclareArgsKey, &cur_args_scope);
+  if (!val_args_scope || !cur_args_scope || (val_args_scope != cur_args_scope))
+    return true;
+
+  *err = Err(node,
+      "Reading a variable defined in the same declare_args() call.");

[brettw, 2016/11/21 17:23:39]

Can you put an extended help message here explaining how to fix the problem (use
a second_declare_args block if you want dependent defaults).

[Dirk Pranke, 2016/11/21 17:39:48]

Will do.

+  return false;
+}
+
 bool EnsureNotProcessingImport(const ParseNode* node,
                                const Scope* scope,
                                Err* err) {
   if (scope->IsProcessingImport()) {
     *err = Err(node, "Not valid from an import.",
         "Imports are for defining defaults, variables, and rules. The\n"
         "appropriate place for this kind of thing is really in a normal\n"
         "BUILD file.");
     return false;
   }
@@ skipping 290 matching lines @@
 
   Introduces the given arguments into the current scope. If they are not
   specified on the command line or in a toolchain's arguments, the default
   values given in the declare_args block will be used. However, these defaults
   will not override command-line values.
 
   See also "gn help buildargs" for an overview.
 
   The precise behavior of declare args is:
 
-   1. The declare_arg block executes. Any variables in the enclosing scope are
-      available for reading.
+   1. The declare_args() block executes. Any variable defined in the enclosing
+      scope is available for reading, but any variable defined earlier in
+      the current scope is not (since the overrides haven't been applied yet).
 
    2. At the end of executing the block, any variables set within that scope
       are saved globally as build arguments, with their current values being
       saved as the "default value" for that argument.
 
    3. User-defined overrides are applied. Anything set in "gn args" now
       overrides any default values. The resulting set of variables is promoted
       to be readable from the following code in the file.
 
   This has some ramifications that may not be obvious:
 
     - You should not perform difficult work inside a declare_args block since
       this only sets a default value that may be discarded. In particular,
       don't use the result of exec_script() to set the default value. If you
       want to have a script-defined default, set some default "undefined" value
       like [], "", or -1, and after the declare_args block, call exec_script if
       the value is unset by the user.
 
-    - Any code inside of the declare_args block will see the default values of
-      previous variables defined in the block rather than the user-overridden
-      value. This can be surprising because you will be used to seeing the
-      overridden value. If you need to make the default value of one arg
-      dependent on the possibly-overridden value of another, write two separate
-      declare_args blocks:
+    - Because you cannot read the value of a variable defined in the same
+      block, if you need to make the default value of one arg depend
+      on the possibly-overridden value of another, write two separate
+      declare_args() blocks:
 
         declare_args() {
           enable_foo = true
         }
         declare_args() {
           # Bar defaults to same user-overridden state as foo.
           enable_bar = enable_foo
         }
 
 Example
@@ skipping 12 matching lines @@
 Value RunDeclareArgs(Scope* scope,
                      const FunctionCallNode* function,
                      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_scope.SetProperty(&kInDeclareArgsKey, &block_scope);
   block->Execute(&block_scope, err);
   if (err->has_error())
     return Value();
 
   // Pass the values from our scope into the Args object for adding to the
   // scope with the proper values (taking into account the defaults given in
   // the block_scope, and arguments passed into the build).
   Scope::KeyValueMap values;
   block_scope.GetCurrentScopeValues(&values);
   scope->settings()->build_settings()->build_args().DeclareArgs(
@@ skipping 679 matching lines @@
   }
 
   // Otherwise it's a no-block function.
   if (!VerifyNoBlockForFunctionCall(function, block, err))
     return Value();
   return found_function->second.no_block_runner(scope, function,
                                                 args.list_value(), err);
 }
 
 }  // namespace functions