Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(49)

Unified Diff: tools/gn/template.cc

Issue 226223006: Template invocation fixes in GN (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 6 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« base/BUILD.gn ('K') | « tools/gn/scope_per_file_provider_unittest.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: tools/gn/template.cc
diff --git a/tools/gn/template.cc b/tools/gn/template.cc
index fa0ccf31c64c4e3a43b5a4a8b1e24d51e87deea6..bf4eb6187804c08f4e2586ba9d9046151cc04aad 100644
--- a/tools/gn/template.cc
+++ b/tools/gn/template.cc
@@ -8,6 +8,7 @@
#include "tools/gn/functions.h"
#include "tools/gn/parse_tree.h"
#include "tools/gn/scope.h"
+#include "tools/gn/scope_per_file_provider.h"
#include "tools/gn/value.h"
Template::Template(const Scope* scope, const FunctionCallNode* def)
@@ -50,17 +51,24 @@ Value Template::Invoke(Scope* scope,
if (err->has_error())
return Value();
- // Set up the scope to run the template. This should be dependent on the
- // closure, but have the "invoker" and "target_name" values injected, and the
- // current dir matching the invoker. We jump through some hoops to avoid
- // copying the invocation scope when setting it in the template scope (since
- // the invocation scope may have large lists of source files in it and could
- // be expensive to copy).
+ // Set up the scope to run the template and set the current directory for the
+ // template (which ScopePerFileProvider uses to base the target-related
+ // variables target_gen_dir and target_out_dir on) to be that of the invoker.
+ // This way, files don't have to be rebased and target_*_dir works the way
+ // people expect (otherwise its to easy to be putting generated files in the
+ // gen dir corresponding to an imported file).
+ Scope template_scope(closure_.get());
+ template_scope.set_source_dir(scope->GetSourceDir());
+
+ ScopePerFileProvider per_file_provider(&template_scope, true);
+
+ // We jump through some hoops to avoid copying the invocation scope when
+ // setting it in the template scope (since the invocation scope may have
+ // large lists of source files in it and could be expensive to copy).
//
// Scope.SetValue will copy the value which will in turn copy the scope, but
// if we instead create a value and then set the scope on it, the copy can
// be avoided.
- Scope template_scope(closure_.get());
const char kInvoker[] = "invoker";
template_scope.SetValue(kInvoker, Value(NULL, scoped_ptr<Scope>()),
invocation);
@@ -73,11 +81,11 @@ Value Template::Invoke(Scope* scope,
Value(invocation, args[0].string_value()),
invocation);
- // Run the template code. Don't check for unused variables since the
- // template could be executed in many different ways and it could be that
- // not all executions use all values in the closure.
+ // Actually run the template code.
Value result =
definition_->block()->ExecuteBlockInScope(&template_scope, err);
+ if (err->has_error())
+ return Value();
// Check for unused variables in the invocation scope. This will find typos
// of things the caller meant to pass to the template but the template didn't
« base/BUILD.gn ('K') | « tools/gn/scope_per_file_provider_unittest.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698