Fix V3OutJsonFile comma emission for nested JSON scopes (#8469) (#8474)

docs/CONTRIBUTORS

@@ -249,6 +249,7 @@ Nathan Myers
 Nick Brereton
 Nikolai Kumar
 Nikolay Puzanov
+Noam Gallmann
 Nolan Poe
 Oleh Maksymenko
 Patrick Creighton

src/V3File.h

@@ -285,7 +285,7 @@ class V3OutJsonFile final : public V3OutFile {
 private:
     std::stack<char> m_scope;  // Stack of ']' and '}'  to close currently open scopes
     std::string m_prefix;  // Prefix emitted before each line in current scope
-    bool m_empty = true;  // Current scope is empty, no comma later
+    std::stack<bool> m_empty;  // Current scope is empty, no comma later
 
 public:
     explicit V3OutJsonFile(const string& filename)
@@ -308,6 +308,8 @@ public:
         puts(m_prefix + "\"" + name + "\": " + type + "\n");
         m_prefix += INDENT;
         m_scope.push(type == '{' ? '}' : ']');
+        if (!m_empty.empty()) m_empty.top() = false;
+        m_empty.push(true);
         return *this;
     }
     V3OutJsonFile& begin(char type = '{') {
@@ -315,6 +317,8 @@ public:
         puts(m_prefix + type + "\n");
         m_prefix += INDENT;
         m_scope.push(type == '{' ? '}' : ']');
+        if (!m_empty.empty()) m_empty.top() = false;
+        m_empty.push(true);
         return *this;
     }
 
@@ -348,8 +352,10 @@ public:
         UASSERT(m_prefix.length() >= strlen(INDENT), "prefix underflow");
         m_prefix.erase(m_prefix.end() - strlen(INDENT), m_prefix.end());
         UASSERT(!m_scope.empty(), "end() without begin()");
+        UASSERT(!m_empty.empty(), "end() without begin()");
         puts("\n" + m_prefix + m_scope.top());
         m_scope.pop();
+        m_empty.pop();
         return *this;
     }
 
@@ -360,8 +366,9 @@ public:
 
 private:
     void comma() {
-        if (!m_empty) puts(",\n");
-        m_empty = true;
+        if (m_empty.empty()) return;
+        if (!m_empty.top()) puts(",\n");
+        m_empty.top() = true;
     }
     V3OutJsonFile& putNamed(const std::string& name, const std::string& value, bool quoted) {
         comma();
@@ -372,7 +379,7 @@ private:
         } else {
             puts(m_prefix + "\"" + name + "\": " + valueQ);
         }
-        m_empty = false;
+        m_empty.top() = false;
         return *this;
     }
 };

test_regress/t/t_sarif_clean.py

@@ -0,0 +1,41 @@
+#!/usr/bin/env python3
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# This program is free software; you can redistribute it and/or modify it
+# under the terms of either the GNU Lesser General Public License Version 3
+# or the Perl Artistic License Version 2.0.
+# SPDX-FileCopyrightText: 2024 Wilson Snyder
+# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
+
+import vltest_bootstrap
+
+test.scenarios('vlt')
+
+test.lint(verilator_flags2=['-Wno-fatal --diagnostics-sarif'],
+          expect_filename=test.golden_filename)
+
+sarif_filename = test.obj_dir + "/" + test.vm_prefix + ".sarif"
+
+# Make sure V3Error meta comments aren't in any outputs
+test.file_grep_not(test.compile_log_filename, r'__WARN')
+test.file_grep_not(sarif_filename, r'__WARN')
+
+test.files_identical(sarif_filename, "t/" + test.name + ".sarif.out", "logfile")
+
+# Check that sarif parses
+nout = test.run_capture("sarif --version", check=False)
+version_match = re.search(r'SARIF tools', nout, re.IGNORECASE)
+if not version_match:
+    test.skip("sarif is not installed")
+
+html_filename = test.obj_dir + "/validation.html"
+
+test.run(cmd=['sarif', 'html', sarif_filename, '--output', html_filename])
+
+# Validator:
+#   https://sarifweb.azurewebsites.net/Validation
+
+# Rewrite
+# test.run(cmd=['sarif copy t/t_sarif.out --output ' + test.obj_dir + '/t_sarif.out.rewrite'])
+
+test.passes()

test_regress/t/t_sarif_clean.sarif.out

@@ -0,0 +1,27 @@
+{
+    "$schema": "https://json.schemastore.org/sarif-2.1.0-rtm.5.json",
+    "version": "###",
+    "runs": [
+        {
+            "tool": {
+                "driver": {
+                    "name": "Verilator",
+                    "version": "###",
+                    "informationUri": "https://verilator.org",
+                    "rules": [
+
+                    ]
+                }
+            },
+            "invocations": [
+                {
+                    "commandLine": "--prefix Vt_sarif_clean --lint-only -Mdir obj_vlt/t_sarif_clean --debug-check --x-assign unique -Wno-fatal --diagnostics-sarif +librescan +notimingchecks +libext+.v -y t +incdir+t +define+TEST_OBJ_DIR=obj_vlt/t_sarif_clean +define+TEST_DUMPFILE=obj_vlt/t_sarif_clean/simx.vcd t/t_sarif_clean.v",
+                    "executionSuccessful": true
+                }
+            ],
+            "results": [
+
+            ]
+        }
+    ]
+}

test_regress/t/t_sarif_clean.v

@@ -0,0 +1,15 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain.
+// SPDX-FileCopyrightText: 2009 Wilson Snyder
+// SPDX-License-Identifier: CC0-1.0
+
+module t (
+  input logic a,
+  input logic b,
+  input logic sel,
+  output logic c);
+
+  assign c = sel ? a : b;
+
+endmodule