From d4f2250848dfce45c9cb89eb16964abc5f1c8aed Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Mon, 28 Aug 2023 13:05:55 -0700
Subject: [PATCH 1/2] work

---
 src/passes/Print.cpp    | 23 +++++++++++++-
 test/lit/debug/full.wat | 67 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+), 1 deletion(-)
 create mode 100644 test/lit/debug/full.wat

diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp
index 3ae7b689996..7ceb4393a41 100644
--- a/src/passes/Print.cpp
+++ b/src/passes/Print.cpp
@@ -170,6 +170,25 @@ struct PrintSExpression : public UnifiedExpressionVisitor<PrintSExpression> {
 
   std::vector<HeapType> heapTypes;
 
+  // Track the print indent so that we can see when it changes. That affects how
+  // we print debug annotations. In particular, we don't want to print repeated
+  // debug locations for children, like this:
+  //
+  //  ;;@ file.cpp:20:4
+  //  (block
+  //    ;; no need to annotate here; children have the parent's location by
+  //    ;; default anyhow
+  //    (nop)
+  //
+  // But we do want to print an annotation even if it repeats if it is not a
+  // child:
+  //
+  //  ;;@ file.cpp:20:4
+  //  (block)
+  //  ;;@ file.cpp:20:4 - this is clearer to annotate, to avoid confusion with
+  //                      the case where there is no debug info on the nop
+  //  (nop)
+  //
   unsigned lastPrintIndent = 0;
 
   // Print type names by saved name or index if we have a module, or otherwise
@@ -2377,7 +2396,9 @@ std::ostream& PrintSExpression::printPrefixedTypes(const char* prefix,
 
 void PrintSExpression::printDebugLocation(
   const Function::DebugLocation& location) {
-  if (lastPrintedLocation == location && indent > lastPrintIndent) {
+  // Do not skip repeated debug info in full mode, for less-confusing debugging:
+  // full mode prints out everything in the most verbose manner.
+  if (lastPrintedLocation == location && indent > lastPrintIndent && !full) {
     return;
   }
   lastPrintedLocation = location;
diff --git a/test/lit/debug/full.wat b/test/lit/debug/full.wat
new file mode 100644
index 00000000000..39d9a54080b
--- /dev/null
+++ b/test/lit/debug/full.wat
@@ -0,0 +1,67 @@
+;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited.
+;; RUN: wasm-opt %s -S -o - | filecheck %s --check-prefix=NRML
+;; RUN: BINARYEN_PRINT_FULL=1 wasm-opt %s -S -o - | filecheck %s --check-prefix=FULL
+
+;;
+;; Compare normal text output with debug info to full mode.
+;;
+;; Full mode does not skip repeated debug info in some cases, see below. It also
+;; annotates the type of each node.
+;;
+
+(module
+  ;; NRML:      (func $a
+  ;; NRML-NEXT:  ;;@ src.cpp:1:2
+  ;; NRML-NEXT:  (block $block
+  ;; NRML-NEXT:   (drop
+  ;; NRML-NEXT:    (i32.const 0)
+  ;; NRML-NEXT:   )
+  ;; NRML-NEXT:   ;;@ src.cpp:3:4
+  ;; NRML-NEXT:   (drop
+  ;; NRML-NEXT:    (i32.const 1)
+  ;; NRML-NEXT:   )
+  ;; NRML-NEXT:   ;;@ src.cpp:3:4
+  ;; NRML-NEXT:   (drop
+  ;; NRML-NEXT:    (i32.const 2)
+  ;; NRML-NEXT:   )
+  ;; NRML-NEXT:  )
+  ;; NRML-NEXT:  ;;@ src.cpp:1:2
+  ;; NRML-NEXT: )
+  ;; FULL:      (func $a
+  ;; FULL-NEXT:  [none] ;;@ src.cpp:1:2
+  ;; FULL-NEXT:  [none](block $block
+  ;; FULL-NEXT:   [none] ;;@ src.cpp:1:2
+  ;; FULL-NEXT:   (drop
+  ;; FULL-NEXT:    [i32] ;;@ src.cpp:1:2
+  ;; FULL-NEXT:    (i32.const 0)
+  ;; FULL-NEXT:   )
+  ;; FULL-NEXT:   [none] ;;@ src.cpp:3:4
+  ;; FULL-NEXT:   (drop
+  ;; FULL-NEXT:    [i32] ;;@ src.cpp:3:4
+  ;; FULL-NEXT:    (i32.const 1)
+  ;; FULL-NEXT:   )
+  ;; FULL-NEXT:   [none] ;;@ src.cpp:3:4
+  ;; FULL-NEXT:   (drop
+  ;; FULL-NEXT:    [i32] ;;@ src.cpp:3:4
+  ;; FULL-NEXT:    (i32.const 2)
+  ;; FULL-NEXT:   )
+  ;; FULL-NEXT:  ) ;; end block block
+  ;; FULL-NEXT:  ;;@ src.cpp:1:2
+  ;; FULL-NEXT: )
+  (func $a
+    ;;@ src.cpp:1:2
+    (block $block
+      (drop (i32.const 0)) ;; this child has the same location as the parent
+                           ;; block, and only in full mode will we print such
+                           ;; repeating info, including on the const child of
+                           ;; the drop
+      ;;@ src.cpp:3:4
+      (drop (i32.const 1))
+      (drop (i32.const 2)) ;; this child has the same location as the sibling
+                           ;; before it, but we print it in normal mode as well
+                           ;; as full mode (as that seems less confusing). in
+                           ;; full mode, however, we also annotate the location
+                           ;; of the const node children of the drops.
+    )
+  )
+)

From 114aa96dd33c642a0a11da012009fa27eddd51a5 Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Tue, 29 Aug 2023 10:33:35 -0700
Subject: [PATCH 2/2] use env in test

---
 test/lit/debug/full.wat | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/lit/debug/full.wat b/test/lit/debug/full.wat
index 39d9a54080b..e8e0b9a9b36 100644
--- a/test/lit/debug/full.wat
+++ b/test/lit/debug/full.wat
@@ -1,6 +1,6 @@
 ;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited.
 ;; RUN: wasm-opt %s -S -o - | filecheck %s --check-prefix=NRML
-;; RUN: BINARYEN_PRINT_FULL=1 wasm-opt %s -S -o - | filecheck %s --check-prefix=FULL
+;; RUN: env BINARYEN_PRINT_FULL=1 wasm-opt %s -S -o - | filecheck %s --check-prefix=FULL
 
 ;;
 ;; Compare normal text output with debug info to full mode.