[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