Fix template invocations in GN.

This passes the right tokens to block begin/end for setup, and changed the
Tokens to be copies instead of pointers.

I separate out the set_defaults function into its own file and document it.

I fixed a bug with template invocation where I did data() on a StringPiece to
pass as a null terminated C-string which doesn't work at all.

R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/22859028

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@218509 0039d316-1c4b-4281-b951-d872f2087c98

tools/gn/BUILD.gn

@@ -38,6 +38,7 @@ static_library("gn_lib") {
     "function_process_file_template.cc",
     "function_read_file.cc",
     "function_set_default_toolchain.cc",
+    "function_set_defaults.cc",
     "function_template.cc",
     "function_toolchain.cc",
     "function_write_file.cc",

tools/gn/function_set_defaults.cc

@@ -0,0 +1,89 @@
+// 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/err.h"
+#include "tools/gn/functions.h"
+#include "tools/gn/parse_tree.h"
+#include "tools/gn/scope.h"
+
+namespace functions {
+
+const char kSetDefaults[] = "set_defaults";
+const char kSetDefaults_Help[] =
+    "set_defaults: Set default values for a target type.\n"
+    "\n"
+    "  set_defaults(<target_type_name>) { <values...> }\n"
+    "\n"
+    "  Sets the default values for a given target type. Whenever\n"
+    "  target_type_name is seen in the future, the values specified in\n"
+    "  set_default's block will be copied into the current scope.\n"
+    "\n"
+    "  When the target type is used, the variable copying is very strict.\n"
+    "  If a variable with that name is already in scope, the build will fail\n"
+    "  with an error.\n"
+    "\n"
+    "  set_defaults can be used for built-in target types (\"executable\",\n"
+    "  \"shared_library\", etc.) and custom ones defined via the \"template\"\n"
+    "  command.\n"
+    "\n"
+    "Example:\n"
+    "  set_defaults(\"static_library\") {\n"
+    "    configs = [ \"//tools/mything:settings\" ]\n"
+    "  }\n"
+    "\n"
+    "  static_library(\"mylib\")\n"
+    "    # The configs will be auto-populated as above. You can remove it if\n"
+    "    # you don't want the default for a particular default:\n"
+    "    configs -= \"//tools/mything:setgings\"\n"
+    "  }\n";
+
+Value RunSetDefaults(Scope* scope,
+                     const FunctionCallNode* function,
+                     const std::vector<Value>& args,
+                     BlockNode* block,
+                     Err* err) {
+  if (!EnsureSingleStringArg(function, args, err))
+    return Value();
+  const std::string& target_type(args[0].string_value());
+
+  // Ensure there aren't defaults already set.
+  //
+  // It might be nice to allow multiple calls set mutate the defaults. The
+  // main case for this is where some local portions of the code want
+  // additional defaults they specify in an imported file.
+  //
+  // Currently, we don't allow imports to clobber anything, so this wouldn't
+  // work. Additionally, allowing this would be undesirable since we don't
+  // want multiple imports to each try to set defaults, since it might look
+  // like the defaults are modified by each one in sequence, while in fact
+  // imports would always clobber previous values and it would be confusing.
+  //
+  // If we wanted this, the solution would be to allow imports to overwrite
+  // target defaults set up by the default build config only. That way there
+  // are no ordering issues, but this would be more work.
+  if (scope->GetTargetDefaults(target_type)) {
+    *err = Err(function->function(),
+               "This target type defaults were already set.");
+    return Value();
+  }
+
+  if (!block) {
+    FillNeedsBlockError(function, err);
+    return Value();
+  }
+
+  // Run the block for the rule invocation.
+  Scope block_scope(scope);
+  block->ExecuteBlockInScope(&block_scope, err);
+  if (err->has_error())
+    return Value();
+
+  // Now copy the values set on the scope we made into the free-floating one
+  // (with no containing scope) used to hold the target defaults.
+  Scope* dest = scope->MakeTargetDefaults(target_type);
+  block_scope.NonRecursiveMergeTo(dest, function, "<SHOULD NOT FAIL>", err);
+  return Value();
+}
+
+}  // namespace functions

tools/gn/functions.cc

@@ -22,12 +22,6 @@
 
 namespace {
 
-void FillNeedsBlockError(const FunctionCallNode* function, Err* err) {
-  *err = Err(function->function(), "This function call requires a block.",
-      "The block's \"{\" must be on the same line as the function "
-      "call's \")\".");
-}
-
 // This is called when a template is invoked. When we see a template
 // declaration, that funciton is RunTemplate.
 Value RunTemplateInvocation(Scope* scope,
@@ -38,9 +32,10 @@ Value RunTemplateInvocation(Scope* scope,
                             Err* err) {
   if (!EnsureNotProcessingImport(invocation, scope, err))
     return Value();
+
   Scope block_scope(scope);
   if (!FillTargetBlockScope(scope, invocation,
-                            invocation->function().value().data(),
+                            invocation->function().value().as_string(),
                             block, args, &block_scope, err))
     return Value();
 
@@ -90,7 +85,7 @@ bool EnsureNotProcessingBuildConfig(const ParseNode* node,
 
 bool FillTargetBlockScope(const Scope* scope,
                           const FunctionCallNode* function,
-                          const char* target_type,
+                          const std::string& target_type,
                           const BlockNode* block,
                           const std::vector<Value>& args,
                           Scope* block_scope,
@@ -122,6 +117,12 @@ bool FillTargetBlockScope(const Scope* scope,
   return true;
 }
 
+void FillNeedsBlockError(const FunctionCallNode* function, Err* err) {
+  *err = Err(function->function(), "This function call requires a block.",
+      "The block's \"{\" must be on the same line as the function "
+      "call's \")\".");
+}
+
 bool EnsureSingleStringArg(const FunctionCallNode* function,
                            const std::vector<Value>& args,
                            Err* err) {
@@ -325,48 +326,6 @@ Value RunImport(Scope* scope,
   return Value();
 }
 
-// set_defaults ----------------------------------------------------------------
-
-const char kSetDefaults[] = "set_defaults";
-const char kSetDefaults_Help[] =
-    "TODO(brettw) write this.";
-
-Value RunSetDefaults(Scope* scope,
-                     const FunctionCallNode* function,
-                     const std::vector<Value>& args,
-                     BlockNode* block,
-                     Err* err) {
-  if (!EnsureSingleStringArg(function, args, err))
-    return Value();
-  const std::string& target_type(args[0].string_value());
-
-  // Ensure there aren't defaults already set.
-  if (scope->GetTargetDefaults(target_type)) {
-    *err = Err(function->function(),
-               "This target type defaults were already set.");
-    return Value();
-  }
-
-  // Execute the block in a new scope that has a parent of the containing
-  // scope.
-  Scope block_scope(scope);
-  if (!FillTargetBlockScope(scope, function,
-                            function->function().value().data(),
-                            block, args, &block_scope, err))
-    return Value();
-
-  // Run the block for the rule invocation.
-  block->ExecuteBlockInScope(&block_scope, err);
-  if (err->has_error())
-    return Value();
-
-  // Now copy the values set on the scope we made into the free-floating one
-  // (with no containing scope) used to hold the target defaults.
-  Scope* dest = scope->MakeTargetDefaults(target_type);
-  block_scope.NonRecursiveMergeTo(dest, function, "<SHOULD NOT FAIL>", err);
-  return Value();
-}
-
 // set_sources_assignment_filter -----------------------------------------------
 
 const char kSetSourcesAssignmentFilter[] = "set_sources_assignment_filter";

tools/gn/functions.h

@@ -283,12 +283,16 @@ bool EnsureNotProcessingBuildConfig(const ParseNode* node,
 // On success, returns true. On failure, sets the error and returns false.
 bool FillTargetBlockScope(const Scope* scope,
                           const FunctionCallNode* function,
-                          const char* target_type,
+                          const std::string& target_type,
                           const BlockNode* block,
                           const std::vector<Value>& args,
                           Scope* block_scope,
                           Err* err);
 
+// Sets the given error to a message explaining that the function call requires
+// a block.
+void FillNeedsBlockError(const FunctionCallNode* function, Err* err);
+
 // Validates that the given function call has one string argument. This is
 // the most common function signature, so it saves space to have this helper.
 // Returns false and sets the error on failure.

tools/gn/gn.gyp

@@ -48,6 +48,7 @@
         'function_process_file_template.cc',
         'function_read_file.cc',
         'function_set_default_toolchain.cc',
+        'function_set_defaults.cc',
         'function_template.cc',
         'function_toolchain.cc',
         'function_write_file.cc',

tools/gn/parse_tree.cc

@@ -139,10 +139,7 @@ void BinaryOpNode::Print(std::ostream& out, int indent) const {
 
 // BlockNode ------------------------------------------------------------------
 
-BlockNode::BlockNode(bool has_scope)
-    : has_scope_(has_scope),
-      begin_token_(NULL),
-      end_token_(NULL) {
+BlockNode::BlockNode(bool has_scope) : has_scope_(has_scope) {
 }
 
 BlockNode::~BlockNode() {
@@ -168,18 +165,19 @@ Value BlockNode::Execute(Scope* containing_scope, Err* err) const {
 }
 
 LocationRange BlockNode::GetRange() const {
-  if (begin_token_ && end_token_) {
-    return begin_token_->range().Union(end_token_->range());
+  if (begin_token_.type() != Token::INVALID &&
+      end_token_.type() != Token::INVALID) {
+    return begin_token_.range().Union(end_token_.range());
+  } else if (!statements_.empty()) {
+    return statements_[0]->GetRange().Union(
+        statements_[statements_.size() - 1]->GetRange());
   }
-  return LocationRange();  // TODO(brettw) indicate the entire file somehow.
+  return LocationRange();
 }
 
 Err BlockNode::MakeErrorDescribing(const std::string& msg,
                                    const std::string& help) const {
-  if (begin_token_)
-    return Err(*begin_token_, msg, help);
-  // TODO(brettw) this should have the beginning of the file in it or something.
-  return Err(Location(NULL, 1, 1), msg, help);
+  return Err(GetRange(), msg, help);
 }
 
 void BlockNode::Print(std::ostream& out, int indent) const {
@@ -413,9 +411,6 @@ Value LiteralNode::Execute(Scope* scope, Err* err) const {
       return Value(this, result_int);
     }
     case Token::STRING: {
-      // TODO(brettw) Unescaping probably needs to be moved & improved.
-      // The input value includes the quotes around the string, strip those
-      // off and unescape.
       Value v(this, Value::STRING);
       ExpandStringLiteral(scope, value_, &v, err);
       return v;

tools/gn/parse_tree.h

@@ -148,8 +148,8 @@ class BlockNode : public ParseNode {
       const std::string& help = std::string()) const OVERRIDE;
   virtual void Print(std::ostream& out, int indent) const OVERRIDE;
 
-  void set_begin_token(const Token* t) { begin_token_ = t; }
-  void set_end_token(const Token* t) { end_token_ = t; }
+  void set_begin_token(const Token& t) { begin_token_ = t; }
+  void set_end_token(const Token& t) { end_token_ = t; }
 
   const std::vector<ParseNode*>& statements() const { return statements_; }
   void append_statement(scoped_ptr<ParseNode> s) {
@@ -163,8 +163,8 @@ class BlockNode : public ParseNode {
   bool has_scope_;
 
   // Tokens corresponding to { and }, if any (may be NULL).
-  const Token* begin_token_;
-  const Token* end_token_;
+  Token begin_token_;
+  Token end_token_;
 
   // Owning pointers, use unique_ptr when we can use C++11.
   std::vector<ParseNode*> statements_;

tools/gn/parser.cc

@@ -397,14 +397,22 @@ scoped_ptr<ParseNode> Parser::ParseStatement() {
 }
 
 scoped_ptr<BlockNode> Parser::ParseBlock() {
-  Consume(Token::LEFT_BRACE, "Expected '{' to start a block.");
+  Token begin_token =
+      Consume(Token::LEFT_BRACE, "Expected '{' to start a block.");
+  if (has_error())
+    return scoped_ptr<BlockNode>();
   scoped_ptr<BlockNode> block(new BlockNode(true));
+  block->set_begin_token(begin_token);
+
   for (;;) {
-    if (Match(Token::RIGHT_BRACE))
+    if (LookAhead(Token::RIGHT_BRACE)) {
+      block->set_end_token(Consume());
       break;
+    }
+
     scoped_ptr<ParseNode> statement = ParseStatement();
     if (!statement)
-      break;
+      return scoped_ptr<BlockNode>();
     block->append_statement(statement.Pass());
   }
   return block.Pass();

tools/gn/token.h

@@ -58,7 +58,8 @@ class Token {
   LocationRange range() const {
     return LocationRange(location_,
                          Location(location_.file(), location_.line_number(),
-                                  location_.char_offset() + value_.size()));
+                                  location_.char_offset() +
+                                      static_cast<int>(value_.size())));
   }
 
   // Helper functions for comparing this token to something.