[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