[template] Correct print_stack_trace() output and add toolchain
print_stack_trace() was walking up both invoker and containing
scopes, causing it to double-print entries.
Instead, it needed to only walk up the invocation scopes, and
for each invocation scope, search it and its containing scopes
to find a template invocation entry.
Change-Id: I7a2228b50f0dae0f4d8abe58f2f24e78bf2ea1d8
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/14040
Reviewed-by: Takuto Ikuta <tikuta@google.com>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Aaron Wood <aaronwood@google.com>
diff --git a/src/gn/functions.cc b/src/gn/functions.cc
index 2b33190..59d97c4 100644
--- a/src/gn/functions.cc
+++ b/src/gn/functions.cc
@@ -1007,7 +1007,10 @@
const std::vector<Value>& args,
Err* err) {
std::string location_str = function->GetRange().begin().Describe(false);
- std::string output = "print_stack_trace() initiated at: " + location_str;
+ std::string toolchain =
+ scope->settings()->toolchain_label().GetUserVisibleName(false);
+ std::string output =
+ "print_stack_trace() initiated at: " + location_str + " using: " + toolchain;
output.push_back('\n');
for (const auto& entry : scope->GetTemplateInvocationEntries()) {
diff --git a/src/gn/functions_unittest.cc b/src/gn/functions_unittest.cc
index 783a22a..6498415 100644
--- a/src/gn/functions_unittest.cc
+++ b/src/gn/functions_unittest.cc
@@ -478,7 +478,7 @@
EXPECT_EQ(
"lala\n"
"42\n"
- "print_stack_trace() initiated at: //test:4\n"
+ "print_stack_trace() initiated at: //test:4 using: //toolchain:default\n"
" foo(\"lala\") //test:6\n"
" print_stack_trace() //test:4\n",
setup.print_output());
@@ -491,10 +491,10 @@
Err err;
input.parsed()->Execute(setup.scope(), &err);
- ASSERT_FALSE(err.has_error()) << err.message();
+ ASSERT_FALSE(err.has_error()) << err.message() << "\n\n" << err.help_text();
EXPECT_EQ(
- "print_stack_trace() initiated at: //test:1\n"
+ "print_stack_trace() initiated at: //test:1 using: //toolchain:default\n"
" print_stack_trace() //test:1\n",
setup.print_output());
}
@@ -520,12 +520,12 @@
Err err;
input.parsed()->Execute(setup.scope(), &err);
- ASSERT_FALSE(err.has_error()) << err.message();
+ ASSERT_FALSE(err.has_error()) << err.message() << "\n\n" << err.help_text();
EXPECT_EQ(
"lala.foo\n"
"42\n"
- "print_stack_trace() initiated at: //test:4\n"
+ "print_stack_trace() initiated at: //test:4 using: //toolchain:default\n"
" baz(\"lala\") //test:11\n"
" foo(\"lala.foo\") //test:7\n"
" print_stack_trace() //test:4\n",
@@ -554,12 +554,12 @@
Err err;
input.parsed()->Execute(setup.scope(), &err);
- ASSERT_FALSE(err.has_error()) << err.message();
+ ASSERT_FALSE(err.has_error()) << err.message() << "\n\n" << err.help_text();
EXPECT_EQ(
"lala.foo\n"
"42\n"
- "print_stack_trace() initiated at: //test:5\n"
+ "print_stack_trace() initiated at: //test:5 using: //toolchain:default\n"
" baz(\"lala\") //test:13\n"
" foo(\"lala.foo\") //test:9\n"
" print_stack_trace() //test:5\n",
@@ -589,15 +589,57 @@
ASSERT_FALSE(input.has_error());
Err err;
input.parsed()->Execute(setup.scope(), &err);
-
- ASSERT_FALSE(err.has_error()) << err.message();
+ ASSERT_FALSE(err.has_error()) << err.message() << "\n\n" << err.help_text();
EXPECT_EQ(
"lala.foo\n"
"42\n"
- "print_stack_trace() initiated at: //test:5\n"
+ "print_stack_trace() initiated at: //test:5 using: //toolchain:default\n"
" baz(\"lala\") //test:15\n"
" foo(\"lala.foo\") //test:10\n"
" print_stack_trace() //test:5\n",
setup.print_output());
}
+
+TEST(Template, PrintStackTraceWithTemplateDefinedWithinATemplate) {
+ TestWithScope setup;
+ TestParseInput input(
+ "template(\"foo\") {\n"
+ " print(target_name)\n"
+ " if (defined(invoker.foo_value)) {\n"
+ " template(\"foo_internal\") {"
+ " print(target_name)\n"
+ " print(invoker.foo_internal_value)\n"
+ " print_stack_trace()\n"
+ " }\n"
+ " foo_internal(target_name+\".internal\") {"
+ " foo_internal_value = invoker.foo_value\n"
+ " }\n"
+ " }\n"
+ "}\n"
+ "template(\"baz\") {\n"
+ " if (invoker.bar == 42) {\n"
+ " foo(\"${target_name}.foo\") {\n"
+ " foo_value = invoker.bar\n"
+ " }\n"
+ " }\n"
+ "}\n"
+ "baz(\"lala\") {\n"
+ " bar = 42\n"
+ "}");
+ ASSERT_FALSE(input.has_error());
+ Err err;
+ input.parsed()->Execute(setup.scope(), &err);
+ ASSERT_FALSE(err.has_error()) << err.message() << "\n\n" << err.help_text();
+
+ EXPECT_EQ(
+ "lala.foo\n"
+ "lala.foo.internal\n"
+ "42\n"
+ "print_stack_trace() initiated at: //test:6 using: //toolchain:default\n"
+ " baz(\"lala\") //test:19\n"
+ " foo(\"lala.foo\") //test:14\n"
+ " foo_internal(\"lala.foo.internal\") //test:8\n"
+ " print_stack_trace() //test:6\n",
+ setup.print_output());
+}
diff --git a/src/gn/scope.cc b/src/gn/scope.cc
index e51282a..24888cf 100644
--- a/src/gn/scope.cc
+++ b/src/gn/scope.cc
@@ -572,25 +572,24 @@
std::move(location)});
}
+const Scope::TemplateInvocationEntry* Scope::FindTemplateInvocationEntry() const {
+ if (template_invocation_entry_)
+ return template_invocation_entry_.get();
+ if (const Scope* scope = containing())
+ return scope->FindTemplateInvocationEntry();
+ return nullptr;
+}
+
void Scope::AppendTemplateInvocationEntries(
std::vector<TemplateInvocationEntry>* out) const {
- // Bare scopes and if/for_each() scopes need to walk up their containing
- // scopes to find previous template invocations. A scope like this within
- // a template invocation will have an "invoker" value, but that invoker will
- // not have an entry, and so both are checked to ensure that the full stack of
- // invocations is captured.
- if (containing())
- containing()->AppendTemplateInvocationEntries(out);
-
- // Template scopes need to walk up invoker to find previous template
- // invocations
const Value* invoker = GetValue("invoker");
if (invoker && invoker->type() == Value::SCOPE)
invoker->scope_value()->AppendTemplateInvocationEntries(out);
- if (template_invocation_entry_)
- out->push_back(*template_invocation_entry_);
+ const TemplateInvocationEntry* entry = FindTemplateInvocationEntry();
+ if (entry)
+ out->push_back(*entry);
}
std::vector<Scope::TemplateInvocationEntry>
diff --git a/src/gn/scope.h b/src/gn/scope.h
index e82c56f..eb38202 100644
--- a/src/gn/scope.h
+++ b/src/gn/scope.h
@@ -369,6 +369,9 @@
// previous template invocations.
void AppendTemplateInvocationEntries(std::vector<TemplateInvocationEntry>* out) const;
+ // Walk up the containing scopes to find a TemplateInvocationEntry.
+ const TemplateInvocationEntry* FindTemplateInvocationEntry() const;
+
// Scopes can have no containing scope (both null), a mutable containing
// scope, or a const containing scope. The reason is that when we're doing
// a new target, we want to refer to the base_config scope which will be read
