Don't dynamically scope GN templates.

tools/gn/function_template.cc

Index: tools/gn/function_template.cc
diff --git a/tools/gn/function_template.cc b/tools/gn/function_template.cc
index f726529616028ca1e4f8b756c77fdd3fb403bd5e..46a2fec30559e87381aa3ed021f4cd18912a780b 100644
--- a/tools/gn/function_template.cc
+++ b/tools/gn/function_template.cc
@@ -6,6 +6,7 @@
 
 #include "tools/gn/parse_tree.h"
 #include "tools/gn/scope.h"
+#include "tools/gn/template.h"
 #include "tools/gn/value.h"
 
 namespace functions {
@@ -14,71 +15,129 @@ const char kTemplate[] = "template";
 const char kTemplate_Help[] =
     "template: Define a template rule.\n"
     "\n"
-    "  A template defines a custom rule name that can expand to one or more\n"
-    "  other rules (typically built-in rules like \"static_library\"). It\n"
+    "  A template defines a custom name that acts like a function. It\n"
     "  provides a way to add to the built-in target types.\n"
     "\n"
     "  The template() function is used to declare a template. To invoke the\n"
     "  template, just use the name of the template like any other target\n"
     "  type.\n"
     "\n"
+    "  Often you will want to declare your template in a special file that\n"
+    "  other files will import (see \"gn help import\") so your template\n"
+    "  rule can be shared across build files.\n"
+    "\n"
     "More details:\n"
     "\n"
-    "  Semantically, the code in the template is stored. When a function\n"
-    "  with the name is called, the block following the invocation is\n"
-    "  executed, *then* your template code is executed. So if the invocation\n"
-    "  sets the |source| variable, for example, that variable will be\n"
-    "  accessible to you when the template code runs.\n"
+    "  When you call template() it creates a closure around all variables\n"
+    "  currently in scope with the code in the template block. When the\n"
+    "  template is invoked, the closure will be executed.\n"
     "\n"
-    "  The template() function does not generate a closure, so the\n"
-    "  environment, current directory, etc. will all be the same as from\n"
-    "  the template is invoked.\n"
+    "  When the template is invoked, the code in the caller is executed and\n"
+    "  passed to the template code as an implicit \"invoker\" variable. The\n"
+    "  template uses this to read state out of the invoking code.\n"
     "\n"
-    "Hints:\n"
+    "  One thing explicitly excluded from the closure is the \"current\n"
+    "  directory\" against which relative file names are resolved. The\n"
+    "  current directory will be that of the invoking code, since typically\n"
+    "  that code specifies the file names. This means all files internal\n"
+    "  to the template should use absolute names.\n"
     "\n"
-    "  If your template expands to more than one target, be sure to name\n"
-    "  the intermediate targets based on the name of the template\n"
-    "  instantiation so that the names are globally unique. The variable\n"
-    "  |target_name| will be this name.\n"
+    "Target naming:\n"
     "\n"
-    "  Likewise, you will always want to generate a target in your template\n"
-    "  with the original |target_name|. Otherwise, invoking your template\n"
-    "  will not actually generate a node in the dependency graph that other\n"
-    "  targets can reference.\n"
+    "  Your template should almost always define a built-in target with the\n"
+    "  name the template invoker specified. For example, if you have an IDL\n"
+    "  template and somebody does:\n"
+    "    idl(\"foo\") {...\n"
+    "  you will normally want this to expand to something defining a\n"
+    "  source_set or static_library named \"foo\" (among other things you may\n"
+    "  need). This way, when another target specifies a dependency on\n"
+    "  \"foo\", the static_library or source_set will be linked.\n"
     "\n"
-    "  Often you will want to declare your template in a special file that\n"
-    "  other files will import (see \"gn help import\") so your template\n"
-    "  rule can be shared across build files.\n"
+    "  It is also important that any other targets your template expands to\n"
+    "  have globally unique names, or you will get collisions.\n"
+    "\n"
+    "  Access the invoking name in your template via the implicit\n"
+    "  \"target_name\" variable. This should also be the basis of how other\n"
+    "  targets that a template expands to to ensure uniquness.\n"
+    "\n"
+    "  A typical example would be a template that defines an action to\n"
+    "  generate some source files, and a source_set to compile that source.\n"
+    "  Your template would name the source_set \"target_name\" because\n"
+    "  that's what you want external targets to depend on to link your code.\n"
+    "  And you would name the action something like \"${target_name}_action\"\n"
+    "  to make it unique. The source set would have a dependency on the\n"
+    "  action to make it run.\n"
     "\n"
     "Example of defining a template:\n"
     "\n"
     "  template(\"my_idl\") {\n"
-    "    # Maps input files to output files, used in both targets below.\n"
+    "    # Be nice and help callers debug problems by checking that the\n"
+    "    # variables the template requires are defined. This gives a nice\n"
+    "    # message rather than giving the user an error about an\n"
+    "    # undefined variable in the file defining the template\n"
+    "    #\n"
+    "    # You can also use defined() to give default values to variables\n"
+    "    # unspecified by the invoker.\n"
+    "    assert(defined(invoker.sources),\n"
+    "           \"Need sources in $target_name listing the idl files.\")\n"
+    "\n"
+    "    # Define a variable containing a source expansion\n"
+    "    # (see \"gn help source_expansion\") that maps input files to\n"
+    "    # output files. It is used in both targets below.\n"
     "    filter = [ \"$target_gen_dir/{{source_name_part}}.cc\",\n"
     "               \"$target_gen_dir/{{source_name_part}}.h\" ]\n"
     "\n"
-    "    # Intermediate target to compile IDL to C source.\n"
+    "    # Intermediate target to convert IDL to C source. Note that the name\n"
+    "    # is based on the name the invoker of the template specified. This\n"
+    "    # way, each time the template is invoked we get a unique\n"
+    "    # intermediate action name (since all target names are in the global\n"
+    "    # scope).\n"
     "    action_foreach(\"${target_name}_code_gen\") {\n"
-    "      # The |sources| will be inherited from the surrounding scope so\n"
-    "      # we don't need to redefine it.\n"
-    "      script = \"foo.py\"\n"
+    "      # Access the scope defined by the invoker via the implicit\n"
+    "      # \"invoker\" variable.\n"
+    "      sources = invoker.sources\n"
+    "\n"
+    "      # Note that we need an absolute path for our script file name.\n"
+    "      # The current directory when executing this code will be that of\n"
+    "      # the invoker (this is why we can use the \"sources\" directly\n"
+    "      # above without having to rebase all of the paths). But if we need\n"
+    "      # to reference a script relative to the template file, we'll need\n"
+    "      # to use an absolute path instead.\n"
+    "      script = \"//tools/idl/idl_code_generator.py\"\n"
     "      outputs = filter  # Variable from above.\n"
     "    }\n"
     "\n"
-    "    # Name the static library the same as the template invocation so\n"
+    "    # Name the source set the same as the template invocation so\n"
     "    # instancing this template produces something that other targets\n"
     "    # can link to in their deps.\n"
-    "    static_library(target_name) {\n"
+    "    source_set(target_name) {\n"
     "      # Generates the list of sources.\n"
     "      # See \"gn help process_file_template\"\n"
-    "      sources = process_file_template(sources, filter)\n"
+    "      sources = process_file_template(invoker.sources, filter)\n"
+    "\n"
+    "      # This target depends on the files produced by the above code gen\n"
+    "      # target.\n"
+    "      deps = [ \":${target_name}_code_gen\" ]\n"
     "    }\n"
     "  }\n"
     "\n"
     "Example of invoking the resulting template:\n"
     "\n"
+    "  # This calls the template code above, defining target_name to be\n"
+    "  # \"foo_idl_files\" and \"invoker\" to be the set of stuff defined in\n"
+    "  # the curly brackets.\n"
     "  my_idl(\"foo_idl_files\") {\n"
+    "    # Goes into the template as \"invoker.sources\".\n"
     "    sources = [ \"foo.idl\", \"bar.idl\" ]\n"
+    "  }\n"
+    "\n"
+    "  # Here is a target that depends on our template.\n"
+    "  executable(\"my_exe\") {\n"
+    "    # Depend on the name we gave the template call above. Internally,\n"
+    "    # this will produce a dependency from executable to the source_set\n"
+    "    # inside the template (since it has this name), which will in turn\n"
+    "    # depend on the code gen action.\n"
+    "    deps = [ \":foo_idl_files\" ]\n"
     "  }\n";
 
 Value RunTemplate(Scope* scope,
@@ -97,16 +156,17 @@ Value RunTemplate(Scope* scope,
     return Value();
   std::string template_name = args[0].string_value();
 
-  const FunctionCallNode* existing_template = scope->GetTemplate(template_name);
+  const Template* existing_template = scope->GetTemplate(template_name);
   if (existing_template) {
     *err = Err(function, "Duplicate template definition.",
                "A template with this name was already defined.");
-    err->AppendSubErr(Err(existing_template->function(),
+    err->AppendSubErr(Err(existing_template->GetDefinitionRange(),
                           "Previous definition."));
     return Value();
   }
 
-  scope->AddTemplate(template_name, function);
+  scope->AddTemplate(template_name,
+                     scoped_ptr<Template>(new Template(scope, function)));
   return Value();
 }