From ceb7dde93881ea50ab6463a8834ae8572476a97d Mon Sep 17 00:00:00 2001 From: Noam Gallmann <47077581+ganoam@users.noreply.github.com> Date: Wed, 23 Sep 2026 19:03:45 +0200 Subject: [PATCH] Fix V3OutJsonFile comma emission for nested JSON scopes (#8469) (#8474) --- docs/CONTRIBUTORS | 1 + src/V3File.h | 15 +++++++--- test_regress/t/t_sarif_clean.out | 0 test_regress/t/t_sarif_clean.py | 41 ++++++++++++++++++++++++++ test_regress/t/t_sarif_clean.sarif.out | 27 +++++++++++++++++ test_regress/t/t_sarif_clean.v | 15 ++++++++++ 6 files changed, 95 insertions(+), 4 deletions(-) create mode 100644 test_regress/t/t_sarif_clean.out create mode 100755 test_regress/t/t_sarif_clean.py create mode 100644 test_regress/t/t_sarif_clean.sarif.out create mode 100644 test_regress/t/t_sarif_clean.v diff --git a/docs/CONTRIBUTORS b/docs/CONTRIBUTORS index fa51db2e1..5fbe6dfbc 100644 --- a/docs/CONTRIBUTORS +++ b/docs/CONTRIBUTORS @@ -249,6 +249,7 @@ Nathan Myers Nick Brereton Nikolai Kumar Nikolay Puzanov +Noam Gallmann Nolan Poe Oleh Maksymenko Patrick Creighton diff --git a/src/V3File.h b/src/V3File.h index 954b0773c..adb6aeb75 100644 --- a/src/V3File.h +++ b/src/V3File.h @@ -285,7 +285,7 @@ class V3OutJsonFile final : public V3OutFile { private: std::stack 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 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; } }; diff --git a/test_regress/t/t_sarif_clean.out b/test_regress/t/t_sarif_clean.out new file mode 100644 index 000000000..e69de29bb diff --git a/test_regress/t/t_sarif_clean.py b/test_regress/t/t_sarif_clean.py new file mode 100755 index 000000000..9bc1179b0 --- /dev/null +++ b/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() diff --git a/test_regress/t/t_sarif_clean.sarif.out b/test_regress/t/t_sarif_clean.sarif.out new file mode 100644 index 000000000..0b830f4b5 --- /dev/null +++ b/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": [ + + ] + } + ] +} diff --git a/test_regress/t/t_sarif_clean.v b/test_regress/t/t_sarif_clean.v new file mode 100644 index 000000000..694674c26 --- /dev/null +++ b/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