Merge origin/master into fix/2544 Resolve formatter_test.cc conflict by keeping both the #2544 MacroBeforeCloseParen regression and the #2542 continuation-comment tests from master.
diff --git a/.github/bin/run-format.sh b/.github/bin/run-format.sh index 3c1c724..d5ccfa1 100755 --- a/.github/bin/run-format.sh +++ b/.github/bin/run-format.sh
@@ -31,10 +31,16 @@ FORMAT_OUT=${TMPDIR:-/tmp}/clang-format-diff.out -# Use the provided Clang format binary, or try to fallback to clang-format-17 -# or clang-format. +# Use the provided Clang format binary, or try clang-format-19 (CI version), +# then older numbered versions, then unversioned clang-format. if [[ ! -v CLANG_FORMAT ]]; then - if command -v "clang-format-17" 2>&1 >/dev/null + if command -v "clang-format-19" 2>&1 >/dev/null + then + CLANG_FORMAT="clang-format-19" + elif command -v "clang-format-18" 2>&1 >/dev/null + then + CLANG_FORMAT="clang-format-18" + elif command -v "clang-format-17" 2>&1 >/dev/null then CLANG_FORMAT="clang-format-17" elif command -v "clang-format" 2>&1 >/dev/null @@ -44,6 +50,7 @@ (echo "-- Missing the clang-format binary! --"; exit 1) fi fi +echo "Using ${CLANG_FORMAT} ($("${CLANG_FORMAT}" --version))" BUILDIFIER=${BUILDIFIER:-buildifier}
diff --git a/.github/bin/smoke-projects.hashes b/.github/bin/smoke-projects.hashes new file mode 100644 index 0000000..5298441 --- /dev/null +++ b/.github/bin/smoke-projects.hashes
@@ -0,0 +1,20 @@ +a12778b40ae905f82eb2c23ea4ac037f98099fae https://github.com/jamieiles/80x86 +4da15979747f326bde2f9869c64e587ce599772c https://github.com/pulp-platform/axi +b48037e28544425839dbd617d45b1a82631bc1a9 https://github.com/bespoke-silicon-group/basejump_stl +f91010f654a5dfd00f83dbe25dbda482218d540b https://github.com/black-parrot/black-parrot +86d09682492176a1f8fd1732e469bc2d309453a3 https://github.com/chipsalliance/caliptra-rtl +f9fbd09655ec6ee457438140c13a168390ebd043 https://github.com/chipsalliance/Cores-VeeR-EH2 +3ac6208ed0210ff66a557dd7dc4e79e34f1f2f4a https://github.com/openhwgroup/cva6 +0602ee4627b10f301298f2673d826cdd6baa9327 https://github.com/ijor/fx68k +8b8ee086aef72e0833b7f0493d9d33f1e4d3c8e2 https://github.com/lowRISC/ibex +a19e629a1879801ffcc6f2e6256ca435c20570f3 https://github.com/steveicarus/ivtest +8e83643c22a3ba7612a9bb9cec93292dad618ab5 https://github.com/trivialmips/nontrivial-mips +00981352642fb65ef2b6c47143a0f6bb31fd4a97 https://github.com/lowRISC/opentitan +491a4dc9bb25c33e0183caaf683102b9ce273bc1 https://github.com/gtaylormb/opl3_fpga +7b65f6ba0bce58d4d859082660123b7100aae975 https://github.com/rsd-devel/rsd +ebb5e3551a9d93c0ee95f0b767dd878b8927e702 https://github.com/syntacore/scr1 +f200eb2ed7b69ac1c6b8eddd47654522aeee5ce8 https://github.com/olofk/serv +c4229f3bd5220e6d3ba8f390e5d09c87e462e9c7 https://github.com/chipsalliance/sv-tests +7e9841ff775ea22fba3f222f7bdfe35668345e00 https://github.com/taichi-ishitani/tnoc +5d72b6618acfddece7f09382a032ccbc05862fdc https://github.com/SymbiFlow/uvm +1c8e05fd1e9a79ceb8b996a0996674122eed086f https://github.com/SymbiFlow/XilinxUnisimLibrary
diff --git a/.github/bin/smoke-test.sh b/.github/bin/smoke-test.sh index ba3ff96..6e4c080 100755 --- a/.github/bin/smoke-test.sh +++ b/.github/bin/smoke-test.sh
@@ -39,6 +39,7 @@ TMPDIR="${TMPDIR:-/tmp}" readonly BASE_TEST_DIR="${TMPDIR}/test/verible-smoke-test" +readonly DEFAULT_HASH_FILE="$(dirname $0)/smoke-projects.hashes" # Write log files to this directory readonly SMOKE_LOGGING_DIR="${SMOKE_LOGGING_DIR:-$BASE_TEST_DIR/error-logs}" @@ -81,29 +82,12 @@ # # There are some known issues which are all recorded in the associative # array below, mapping them to Verible issue tracker numbers. -# TODO(hzeller): there should be a configuration file that contains two -# columns: URL + hash, so that we can fetch a particular known version not -# a moving target. -readonly TEST_GIT_PROJECTS="https://github.com/lowRISC/ibex \ - https://github.com/lowRISC/opentitan \ - https://github.com/chipsalliance/sv-tests \ - https://github.com/chipsalliance/Cores-VeeR-EH2 \ - https://github.com/chipsalliance/caliptra-rtl \ - https://github.com/openhwgroup/cva6 \ - https://github.com/SymbiFlow/uvm \ - https://github.com/taichi-ishitani/tnoc \ - https://github.com/ijor/fx68k \ - https://github.com/jamieiles/80x86 \ - https://github.com/SymbiFlow/XilinxUnisimLibrary \ - https://github.com/black-parrot/black-parrot - https://github.com/steveicarus/ivtest \ - https://github.com/trivialmips/nontrivial-mips \ - https://github.com/pulp-platform/axi \ - https://github.com/rsd-devel/rsd \ - https://github.com/syntacore/scr1 \ - https://github.com/olofk/serv \ - https://github.com/bespoke-silicon-group/basejump_stl \ - https://github.com/gtaylormb/opl3_fpga" +readonly PROJECT_HASHES_FILE="${1:-${DEFAULT_HASH_FILE}}" + +if [ ! -f "${PROJECT_HASHES_FILE}" ]; then + echo "Project hashes file not found: ${PROJECT_HASHES_FILE}" + exit 1 +fi ## # Some of the files in the projects will have issues. @@ -135,24 +119,24 @@ ExpectedFailCount[syntax:ibex]=13 ExpectedFailCount[lint:ibex]=13 -ExpectedFailCount[project:ibex]=218 -ExpectedFailCount[preprocessor:ibex]=397 +ExpectedFailCount[project:ibex]=224 +ExpectedFailCount[preprocessor:ibex]=403 ExpectedFailCount[syntax:opentitan]=88 ExpectedFailCount[lint:opentitan]=88 -ExpectedFailCount[project:opentitan]=1068 +ExpectedFailCount[project:opentitan]=1077 ExpectedFailCount[formatter:opentitan]=0 -ExpectedFailCount[preprocessor:opentitan]=3074 +ExpectedFailCount[preprocessor:opentitan]=3017 ExpectedFailCount[syntax:sv-tests]=74 ExpectedFailCount[lint:sv-tests]=73 ExpectedFailCount[project:sv-tests]=176 ExpectedFailCount[preprocessor:sv-tests]=128 -ExpectedFailCount[syntax:caliptra-rtl]=39 -ExpectedFailCount[lint:caliptra-rtl]=38 -ExpectedFailCount[project:caliptra-rtl]=453 -ExpectedFailCount[preprocessor:caliptra-rtl]=911 +ExpectedFailCount[syntax:caliptra-rtl]=41 +ExpectedFailCount[lint:caliptra-rtl]=40 +ExpectedFailCount[project:caliptra-rtl]=474 +ExpectedFailCount[preprocessor:caliptra-rtl]=993 ExpectedFailCount[syntax:Cores-VeeR-EH2]=2 ExpectedFailCount[lint:Cores-VeeR-EH2]=2 @@ -164,10 +148,10 @@ ExpectedFailCount[project:cva6]=84 ExpectedFailCount[preprocessor:cva6]=140 -ExpectedFailCount[syntax:uvm]=0 -ExpectedFailCount[lint:uvm]=0 -ExpectedFailCount[project:uvm]=40 -ExpectedFailCount[preprocessor:uvm]=126 +ExpectedFailCount[syntax:uvm]=1 +ExpectedFailCount[lint:uvm]=1 +ExpectedFailCount[project:uvm]=41 +ExpectedFailCount[preprocessor:uvm]=128 ExpectedFailCount[syntax:tnoc]=3 ExpectedFailCount[lint:tnoc]=3 @@ -187,9 +171,9 @@ ExpectedFailCount[project:black-parrot]=172 ExpectedFailCount[preprocessor:black-parrot]=173 -ExpectedFailCount[syntax:ivtest]=117 -ExpectedFailCount[lint:ivtest]=117 -ExpectedFailCount[project:ivtest]=146 +ExpectedFailCount[syntax:ivtest]=116 +ExpectedFailCount[lint:ivtest]=116 +ExpectedFailCount[project:ivtest]=145 ExpectedFailCount[preprocessor:ivtest]=26 ExpectedFailCount[syntax:nontrivial-mips]=2 @@ -237,11 +221,10 @@ expected_count_key="${TOOL_SHORT_NAME}:${PROJECT_NAME}" if [[ -v ExpectedFailCount[${expected_count_key}] ]]; then expected_count=${ExpectedFailCount[${expected_count_key}]} - # We allow some 5% deviation from expected values before we complain loudly - local GRACE_VALUE=$(( ${expected_count} / 20 )) - if [ ${GRACE_VALUE} -lt 2 ]; then - GRACE_VALUE=2 - fi + + # Since we have a fixed set of projects and hashes, we don't allow any + # derivation from the expected value (used to be 5%) + local GRACE_VALUE=0 if [ ${OBSERVED_NONZERO_COUNT} -gt ${expected_count} ] ; then local ALLOWED_UP_TO=$((${expected_count} + ${GRACE_VALUE})) if [ ${OBSERVED_NONZERO_COUNT} -gt ${ALLOWED_UP_TO} ]; then @@ -254,6 +237,7 @@ elif [ ${OBSERVED_NONZERO_COUNT} -lt ${expected_count} ] ; then echo "::notice:: 🎉 Yay, reduced non-zero exit count ${expected_count} -> ${OBSERVED_NONZERO_COUNT}" echo "Set ExpectedFailCount[${TOOL_SHORT_NAME}:${PROJECT_NAME}]=${OBSERVED_NONZERO_COUNT}" + return 1 # we still want the exact value to be recorded so actively fail fi else if [ ${OBSERVED_NONZERO_COUNT} -gt 0 ] ; then @@ -270,11 +254,14 @@ # # First parameter : project name # Second parameter: name of file containing a list of {System}Verilog files +# Third parameter : git URL +# Fourth parameter: git hash (optional, defaults to master) function run_smoke_test() { local PROJECT_FILE_LIST=${TMPDIR}/filelist.$$.list local PROJECT_NAME=$1 local FILELIST=$2 local GIT_URL=$3 + local GIT_HASH=${4:-master} local NUM_FILES=$(wc -l < ${FILELIST}) local result=0 @@ -355,7 +342,7 @@ else # This is an so far unknown issue echo "::error:: 😱 ${single_file}: crash exit code $EXIT_CODE for $tool" - echo "Input File URL: ${GIT_URL}/blob/master/$(echo $single_file | cut -d/ -f6-)" + echo "Input File URL: ${GIT_URL}/blob/${GIT_HASH}/$(echo $single_file | cut -d/ -f6-)" head -15 ${PROJECT_FILE_TOOL_OUT} # Might be useful in this case result=$((${result} + 1)) fi @@ -388,18 +375,20 @@ bazel build ${BAZEL_BUILD_OPTIONS} :install-binaries & # While compiling, run potentially slow network ops -for git_project in ${TEST_GIT_PROJECTS} ; do - PROJECT_NAME="$(basename $git_project)" +while read -r git_hash git_project _; do + [[ -z "${git_hash}" || "${git_hash}" =~ ^# ]] && continue + PROJECT_NAME="$(basename "${git_project}")" PROJECT_DIR="${BASE_TEST_DIR}/${PROJECT_NAME}" - git clone ${git_project} ${PROJECT_DIR} 2>/dev/null & -done + ( git clone "${git_project}" "${PROJECT_DIR}" && git -C "${PROJECT_DIR}" checkout -q "${git_hash}" ) 2>/dev/null & +done < "${PROJECT_HASHES_FILE}" echo "base test dir ${BASE_TEST_DIR}; writing logs to ${SMOKE_LOGGING_DIR}" echo "Waiting... for compilation and project download finished" wait -for git_project in ${TEST_GIT_PROJECTS} ; do - PROJECT_NAME="$(basename $git_project)" +while read -r git_hash git_project _; do + [[ -z "${git_hash}" || "${git_hash}" =~ ^# ]] && continue + PROJECT_NAME="$(basename "${git_project}")" PROJECT_DIR="${BASE_TEST_DIR}/${PROJECT_NAME}" # Already cloned above @@ -412,14 +401,14 @@ FILELIST="${PROJECT_DIR}/verible.filelist" find "${PROJECT_DIR}" -name "*.sv" -o -name "*.svh" -o -name "*.v" | sort > ${FILELIST} - run_smoke_test "${PROJECT_NAME}" "${FILELIST}" "${git_project}" + run_smoke_test "${PROJECT_NAME}" "${FILELIST}" "${git_project}" "${git_hash}" status_sum=$((${status_sum} + $?)) echo -done +done < "${PROJECT_HASHES_FILE}" echo "::endgroup::" -echo "There were a total of ${status_sum} new, undocumented issues." +echo "There were a total of ${status_sum} mismatches" # Let's see if there are any issues that are fixed in the meantime. if [ "${#KnownIssue[@]}" -ne 0 ]; then
diff --git a/.github/workflows/verible-ci.yml b/.github/workflows/verible-ci.yml index 5f03f2f..802a7a3 100644 --- a/.github/workflows/verible-ci.yml +++ b/.github/workflows/verible-ci.yml
@@ -34,6 +34,11 @@ with: fetch-depth: 0 + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version: 'stable' + - name: Install Dependencies run: | sudo apt-get install clang-format-19
diff --git a/verible/common/util/BUILD b/verible/common/util/BUILD index 48a2105..db499b4 100644 --- a/verible/common/util/BUILD +++ b/verible/common/util/BUILD
@@ -22,7 +22,6 @@ hdrs = ["auto-pop-stack.h"], deps = [ ":logging", - "@abseil-cpp//absl/base:core_headers", ], )
diff --git a/verible/common/util/auto-pop-stack.h b/verible/common/util/auto-pop-stack.h index 024ef05..58628eb 100644 --- a/verible/common/util/auto-pop-stack.h +++ b/verible/common/util/auto-pop-stack.h
@@ -18,7 +18,6 @@ #include <cstddef> #include <vector> -#include "absl/base/attributes.h" #include "verible/common/util/logging.h" namespace verible { @@ -29,7 +28,7 @@ // implementing algorithms based on stack data structures like building // inheritance stack of traversed tree. template <typename T> -class AutoPopStack { +class [[maybe_unused]] AutoPopStack { public: using value_type = T; using this_type = AutoPopStack<value_type>; @@ -105,7 +104,7 @@ private: stack_type stack_; -} ABSL_ATTRIBUTE_UNUSED; +}; } // namespace verible
diff --git a/verible/verilog/analysis/symbol-table_test.cc b/verible/verilog/analysis/symbol-table_test.cc index e15ec5f..3e6b1a6 100644 --- a/verible/verilog/analysis/symbol-table_test.cc +++ b/verible/verilog/analysis/symbol-table_test.cc
@@ -73,8 +73,7 @@ const auto found_##dest(map.find(key)); /* iterator */ \ ASSERT_NE(found_##dest, map.end()) \ << "No element at \"" << key << "\" in " #map; \ - const auto &dest ABSL_ATTRIBUTE_UNUSED( \ - found_##dest->second); /* mapped_type */ + const auto &dest [[maybe_unused]] (found_##dest->second); /* mapped_type */ // Assert that container is not empty, and reference its first element. // Works on any container type with .begin(). @@ -103,7 +102,7 @@ << "No symbol at \"" << key << "\" in " << ScopePathPrinter{scope}; \ EXPECT_EQ(found_##dest->first, key); \ const SymbolTableNode &dest(found_##dest->second); \ - const SymbolInfo &dest##_info ABSL_ATTRIBUTE_UNUSED(dest.Value()) + const SymbolInfo &dest##_info [[maybe_unused]] (dest.Value()) // For SymbolInfo::references_map_view_type only: Assert that there is exactly // one element at 'key' in 'map' and assign it to 'dest' (DependentReferences).
diff --git a/verible/verilog/formatting/BUILD b/verible/verilog/formatting/BUILD index 6150594..6431226 100644 --- a/verible/verilog/formatting/BUILD +++ b/verible/verilog/formatting/BUILD
@@ -163,7 +163,6 @@ "//verible/common/util:expandable-tree-view", "//verible/common/util:interval", "//verible/common/util:interval-set", - "//verible/common/util:iterator-range", "//verible/common/util:logging", "//verible/common/util:spacer", "//verible/common/util:tree-operations", @@ -175,7 +174,6 @@ "//verible/verilog/analysis:verilog-equivalence", "//verible/verilog/parser:verilog-token-enum", "//verible/verilog/preprocessor:verilog-preprocess", - "@abseil-cpp//absl/base:core_headers", "@abseil-cpp//absl/log:die_if_null", "@abseil-cpp//absl/status", "@abseil-cpp//absl/status:statusor",
diff --git a/verible/verilog/formatting/formatter.cc b/verible/verilog/formatting/formatter.cc index 9893b60..9bd3dd2 100644 --- a/verible/verilog/formatting/formatter.cc +++ b/verible/verilog/formatting/formatter.cc
@@ -26,7 +26,6 @@ #include <string_view> #include <vector> -#include "absl/base/attributes.h" #include "absl/log/die_if_null.h" #include "absl/status/status.h" #include "absl/status/statusor.h" @@ -50,7 +49,6 @@ #include "verible/common/util/expandable-tree-view.h" #include "verible/common/util/interval-set.h" #include "verible/common/util/interval.h" -#include "verible/common/util/iterator-range.h" #include "verible/common/util/logging.h" #include "verible/common/util/spacer.h" #include "verible/common/util/tree-operations.h" @@ -749,15 +747,24 @@ case verible::SpacingDecision::kPreserve: { if (token.before.preserved_space_start != verible::string_view_null_iterator()) { - *column += token.OriginalLeadingSpaces().length(); + const std::string_view leading = token.OriginalLeadingSpaces(); + const auto last_nl = leading.find_last_of('\n'); + if (last_nl == std::string_view::npos) { + *column += leading.length(); + } else { + // Reset column after the last newline, same as FormattedToken + // emit (GitHub issue 2542). + *column = static_cast<int>(leading.length() - last_nl - 1); + } } else { *column += token.before.spaces; } break; } case verible::SpacingDecision::kWrap: - *column = 0; - ABSL_FALLTHROUGH_INTENDED; + // Newline then only the wrap indent (same as FormattedToken emit). + *column = token.before.spaces; + break; case verible::SpacingDecision::kAlign: case verible::SpacingDecision::kAppend: *column += token.before.spaces; @@ -766,6 +773,15 @@ } static int CalculateEolCommentColumn(const verible::FormattedExcerpt &line) { + // Compute the starting column of the trailing EOL comment the same way + // FormattedExcerpt::FormattedText emits spaces, including: + // * wrap indents (SpacingDecision::kWrap), and + // * preserved leading whitespace that may contain newlines (common when + // an original line break is kept). Counting those newlines as width + // made continuation comments land on the wrong column and fail to + // converge on re-format (GitHub issue 2542). + if (line.Tokens().empty()) return 0; + int column = 0; const auto &front = line.Tokens().front(); @@ -777,12 +793,16 @@ } column += front.token->text().length(); - for (const auto &ftoken : verible::make_range(line.Tokens().begin() + 1, - line.Tokens().end() - 1)) { + const auto &tokens = line.Tokens(); + for (size_t i = 1; i < tokens.size(); ++i) { + const auto &ftoken = tokens[i]; AdjustColumnUsingTokenSpacing(ftoken, &column); - column += ftoken.token->text().length(); + // Do not add the last token's length: that is the EOL comment whose + // starting column we want. + if (i + 1 < tokens.size()) { + column += ftoken.token->text().length(); + } } - AdjustColumnUsingTokenSpacing(line.Tokens().back(), &column); CHECK_GE(column, 0); return column;
diff --git a/verible/verilog/formatting/formatter_test.cc b/verible/verilog/formatting/formatter_test.cc index 5501c58..427ee90 100644 --- a/verible/verilog/formatting/formatter_test.cc +++ b/verible/verilog/formatting/formatter_test.cc
@@ -19398,6 +19398,50 @@ EXPECT_THAT(stream.str(), testing::HasSubstr("`TOKEN_BYTE")); } +// Regression for https://github.com/chipsalliance/verible/issues/2542: +// Continuation EOL comments after a wrapped assign must keep a stable column +// across re-format (convergence). +TEST(FormatterEndToEndTest, ContinuationCommentAfterWrappedAssignConverges) { + static constexpr FormatterTestCase kTestCases[] = { + {// Comments originally column-aligned after a wrapped assign + "module m;\n" + " assign status_ur = !(status_sc || status_ca ||\n" + " status_crs); // Completions with a Reserved Completion\n" + " // Status value are treated as UR\n" + "endmodule\n", + "module m;\n" + " assign status_ur =\n" + " !(status_sc || status_ca || status_crs); // Completions with a " + "Reserved Completion\n" + " // Status value are " + "treated as UR\n" + "endmodule\n"}, + {// Previously mis-aligned continuation is not treated as a continuation + // (column delta > 1) and must still converge + "module m;\n" + " assign status_ur = !(status_sc || status_ca ||\n" + " status_crs); // Completions with a Reserved Completion\n" + " " + "// Status value are treated as UR\n" + "endmodule\n", + "module m;\n" + " assign status_ur =\n" + " !(status_sc || status_ca || status_crs); // Completions with a " + "Reserved Completion\n" + " // Status value are treated as UR\n" + "endmodule\n"}, + }; + FormatStyle style; // default column_limit (100) + for (const auto &test_case : kTestCases) { + VLOG(1) << "code-to-format:\n" << test_case.input << "<EOF>"; + std::ostringstream stream; + const auto status = + FormatVerilog(test_case.input, "<filename>", style, stream); + EXPECT_OK(status) << status.message(); + EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input; + } +} + // Regression for https://github.com/chipsalliance/verible/issues/2540: // Trailing EOL comment after `end` before `else if` must not change whether // the else-if assignment stays on one line across re-format (convergence). @@ -20977,6 +21021,114 @@ } } +// Regression for https://github.com/chipsalliance/verible/issues/2008 +// (also https://github.com/chipsalliance/verible/issues/2474 and +// https://github.com/chipsalliance/verible/issues/2063): +// Non-ANSI "input wire signed" used to abort in the tree-unwrapper because +// the CST visited "signed" before "wire", which is the reverse of source +// order. +TEST(FormatterEndToEndTest, NonAnsiWireSignedModulePortDoesNotAbort) { + static constexpr FormatterTestCase kTestCases[] = { + {// Original issue #2008 sample + "module uut( sig1 );\n" + "\n" + "input wire signed [15:0] sig1;\n" + "\n" + "endmodule\n", + "module uut (\n" + " sig1\n" + ");\n" + "\n" + " input wire signed [15:0] sig1;\n" + "\n" + "endmodule\n"}, + {// Issue #2474 sample + "module myModule (\n" + " myinput\n" + ");\n" + "input wire signed [7:0] myInput;\n" + "endmodule\n", + "module myModule (\n" + " myinput\n" + ");\n" + " input wire signed [7:0] myInput;\n" + "endmodule\n"}, + {// Issue #2063 sample: signed wire with no packed dimensions + "module top(a);\n" + " input wire signed a;\n" + "endmodule\n", + "module top (\n" + " a\n" + ");\n" + " input wire signed a;\n" + "endmodule\n"}, + {// Same production with logic instead of wire + "module uut(sig1);\n" + "input logic signed [15:0] sig1;\n" + "endmodule\n", + "module uut (\n" + " sig1\n" + ");\n" + " input logic signed [15:0] sig1;\n" + "endmodule\n"}, + {// output / inout net types + "module uut(sig1, sig2);\n" + "output wire signed [15:0] sig1;\n" + "inout wire signed [7:0] sig2;\n" + "endmodule\n", + "module uut (\n" + " sig1,\n" + " sig2\n" + ");\n" + " output wire signed [15:0] sig1;\n" + " inout wire signed [7:0] sig2;\n" + "endmodule\n"}, + {// unsigned is the same production + "module uut(sig1);\n" + "input wire unsigned [15:0] sig1;\n" + "endmodule\n", + "module uut (\n" + " sig1\n" + ");\n" + " input wire unsigned [15:0] sig1;\n" + "endmodule\n"}, + {// ANSI form already worked; keep as a regression + "module uut(input wire signed [15:0] sig1);\n" + "endmodule\n", + "module uut (\n" + " input wire signed [15:0] sig1\n" + ");\n" + "endmodule\n"}, + {// Non-ANSI without signed still works + "module uut(sig1);\n" + "input wire [15:0] sig1;\n" + "endmodule\n", + "module uut (\n" + " sig1\n" + ");\n" + " input wire [15:0] sig1;\n" + "endmodule\n"}, + {// Non-ANSI signed without net type still works + "module uut(sig1);\n" + "input signed [15:0] sig1;\n" + "endmodule\n", + "module uut (\n" + " sig1\n" + ");\n" + " input signed [15:0] sig1;\n" + "endmodule\n"}, + }; + FormatStyle style; // default column_limit (100) + for (const auto &test_case : kTestCases) { + VLOG(1) << "code-to-format:\n" << test_case.input << "<EOF>"; + std::ostringstream stream; + const auto status = + FormatVerilog(test_case.input, "<filename>", style, stream); + EXPECT_OK(status) << status.message(); + EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input; + } +} + } // namespace } // namespace formatter } // namespace verilog
diff --git a/verible/verilog/parser/verilog.y b/verible/verilog/parser/verilog.y index f1c24a8..a6de1a8 100644 --- a/verible/verilog/parser/verilog.y +++ b/verible/verilog/parser/verilog.y
@@ -5623,15 +5623,23 @@ | port_direction signed_unsigned_opt list_of_module_item_identifiers ';' { $$ = MakeTaggedNode(N::kModulePortDeclaration, $1, MakeDataType($2, nullptr, nullptr), $3, $4);} /* implicit type */ + /* Keep net/var type before signing so CST leaf order matches source + * ("wire signed", not "signed" then "wire"). The formatter walks CST + * order and CHECK-fails when a later leaf appears earlier in the file. + * Wrap as kDataTypePrimitive like the TK_bit rule so + * GetBaseTypeFromDataType still treats child 1 as the type. + */ | port_direction port_net_type signed_unsigned_opt decl_dimensions_opt list_of_identifiers_unpacked_dimensions ';' { $$ = MakeTaggedNode(N::kModulePortDeclaration, $1, - MakeDataType($3, ForwardChildren($2), MakePackedDimensionsNode($4)), + MakeDataType(MakeTaggedNode(N::kDataTypePrimitive, $2, $3), + MakePackedDimensionsNode($4)), $5, $6); } | dir var_type signed_unsigned_opt decl_dimensions_opt list_of_port_identifiers ';' { $$ = MakeTaggedNode(N::kModulePortDeclaration, $1, - MakeDataType($3, ForwardChildren($2), MakePackedDimensionsNode($4)), + MakeDataType(MakeTaggedNode(N::kDataTypePrimitive, $2, $3), + MakePackedDimensionsNode($4)), $5, $6); } | port_direction TK_bit signed_unsigned_opt decl_dimensions_opt list_of_identifiers_unpacked_dimensions ';'