Merge pull request #2534 from EylonKrause/fix/port-name-suffix-ref-underscore
port-name-suffix: handle `ref` direction and all-underscore names
diff --git a/.bant-macros b/.bant-macros
new file mode 100644
index 0000000..c62c397
--- /dev/null
+++ b/.bant-macros
@@ -0,0 +1,23 @@
+# -*- Python -*-
+# https://github.com/hzeller/bant#macros
+#
+# Tell bant (that is not looking at *.bzl files) how custom rules behave, just
+# enough for it to see through them to provide features such as dwyu.
+
+record_recovered_syntax_errors = genrule(
+ name = name,
+ srcs = [src],
+ outs = [out],
+)
+
+genlex = genrule(
+ name = name,
+ srcs = [src],
+ outs = [out],
+)
+
+genyacc = genrule(
+ name = name,
+ srcs = [src],
+ outs = [header_out, source_out] + extra_outs,
+)
diff --git a/.bazelrc b/.bazelrc
index 04d1f38..301b564 100644
--- a/.bazelrc
+++ b/.bazelrc
@@ -10,29 +10,38 @@
build --workspace_status_command="bash bazel/build-version.sh"
# Systems with gcc or clang
-common:unix --cxxopt=-xc++ --host_cxxopt=-xc++ --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 --client_env=BAZEL_CXXOPTS=-std=c++17
+common:unix --cxxopt=-xc++ --host_cxxopt=-xc++
+common:unix --cxxopt=-std=c++20 --host_cxxopt=-std=c++20
common:linux --config=unix
common:freebsd --config=unix --linkopt=-lm --host_linkopt=-lm
common:openbsd --config=unix --linkopt=-lm --host_linkopt=-lm
common:macos --config=unix
-# https://github.com/abseil/abseil-cpp/issues/848
-# https://github.com/bazelbuild/bazel/issues/4341#issuecomment-758361769
-common:macos --features=-supports_dynamic_linker --linkopt=-framework --linkopt=CoreFoundation --host_linkopt=-framework --host_linkopt=CoreFoundation
+# Applies to both clang-cl and native MSVC builds.
+common:windows --cxxopt=/std:c++20 --host_cxxopt=/std:c++20 --client_env=BAZEL_CXXOPTS=/std:c++20
+# Actions (e.g. the genrules invoking win_flex.exe/win_bison.exe) need the
+# invoking shell's PATH to find tools installed.
+common:windows --action_env=PATH --host_action_env=PATH
+# Protobuf 31 has [MSVC deprecated](https://github.com/protocolbuffers/protobuf/issues/20085) and need this option to compile
+# The deprecation was later reverted thanks to improvements in bazel 8 and will not be necessary with newer protobuf.
+common:msvc --define=protobuf_allow_msvc=true
-# Use clang-cl by default on Windows. MSVC has some issues with the codebase,
-# so we focus the effort for now is to have a Windows Verible compiled with
-# clang-cl before fixing the issues unique to MSVC.
-common:windows --extra_toolchains=@local_config_cc//:cc-toolchain-x64_windows-clang-cl --extra_execution_platforms=//:x64_windows-clang-cl
-common:windows --compiler=clang-cl --cxxopt=/std:c++17 --host_cxxopt=/std:c++17 --client_env=BAZEL_CXXOPTS=/std:c++17
+# Opt-in: build with clang-cl instead of the natively autoconfigured MSVC
+# with bazel --config=clang-cl
+common:clang-cl --extra_toolchains=@local_config_cc//:cc-toolchain-x64_windows-clang-cl --extra_execution_platforms=//:x64_windows-clang-cl
+common:clang-cl --compiler=clang-cl
-build --cxxopt="-Wno-unknown-warning-option" --host_cxxopt="-Wno-unknown-warning-option"
+common:unix --cxxopt="-Wno-unknown-warning-option" --host_cxxopt="-Wno-unknown-warning-option"
+common:clang-cl --cxxopt="-Wno-unknown-warning-option" --host_cxxopt="-Wno-unknown-warning-option"
# TODO: this looks like benign where it happens but to be explored further.
-build --cxxopt="-Wno-dangling-reference" --host_cxxopt="-Wno-dangling-reference"
+common:unix --cxxopt="-Wno-dangling-reference" --host_cxxopt="-Wno-dangling-reference"
+common:clang-cl --cxxopt="-Wno-dangling-reference" --host_cxxopt="-Wno-dangling-reference"
# Newer bisons create an unused label.
-build --cxxopt="-Wno-unused-label" --host_cxxopt="-Wno-unused-label"
+common:unix --cxxopt="-Wno-unused-label" --host_cxxopt="-Wno-unused-label"
+common:clang-cl --cxxopt="-Wno-unused-label" --host_cxxopt="-Wno-unused-label"
# c++20 warning on protobuf 28.1
-build --cxxopt="-Wno-missing-requires" --host_cxxopt="-Wno-missing-requires"
+common:unix --cxxopt="-Wno-missing-requires" --host_cxxopt="-Wno-missing-requires"
+common:clang-cl --cxxopt="-Wno-missing-requires" --host_cxxopt="-Wno-missing-requires"
# For 3rd party code: Disable warnings entirely.
# They are not actionable and just create noise.
diff --git a/.bazelversion b/.bazelversion
deleted file mode 100644
index 93c8dda..0000000
--- a/.bazelversion
+++ /dev/null
@@ -1 +0,0 @@
-7.6.0
diff --git a/.github/bin/get-bant-path.sh b/.github/bin/get-bant-path.sh
index 9011aea..f1a0cac 100755
--- a/.github/bin/get-bant-path.sh
+++ b/.github/bin/get-bant-path.sh
@@ -21,8 +21,7 @@
# Bant not given, compile from bzlmod dep.
if [ "${BANT}" = "needs-to-be-compiled-locally" ]; then
- "${BAZEL}" build -c opt --cxxopt=-std=c++20 @bant//bant:bant 2>/dev/null
- BANT=$(realpath bazel-bin/external/bant*/bant/bant | tail -1)
+ BANT="$("${BAZEL}" run -c opt --cxxopt=-std=c++20 --run_under='echo' @bant//bant:bant 2>/dev/null)"
fi
echo $BANT
diff --git a/.github/bin/make-compilation-db.sh b/.github/bin/make-compilation-db.sh
index e35a55d..5038893 100755
--- a/.github/bin/make-compilation-db.sh
+++ b/.github/bin/make-compilation-db.sh
@@ -19,11 +19,12 @@
BAZEL=${BAZEL:-bazel}
BANT=$($(dirname $0)/get-bant-path.sh)
-BAZEL_OPTS="-c opt --noshow_progress"
+BAZEL_OPTS="-c opt --noshow_progress --remote_download_outputs=all"
+
# Bazel-build all targets that generate files, so that they can be
# seen in dependency analysis.
-${BAZEL} build -k ${BAZEL_OPTS} $(${BANT} list-targets \
- | awk '/genrule|cc_proto_library|genlex|genyacc/ {print $3}')
+${BAZEL} build -k ${BAZEL_OPTS} $(${BANT} list-targets ... \
+ -g 'genrule|cc_proto_library|genlex|genyacc' -c3)
# Some selected targets to trigger all dependency fetches from MODULE.bazel
# verilog-y-final to create a header, kzip creator to trigger build of any.pb.h
@@ -41,8 +42,3 @@
for d in bazel-out/../../../external/*flex*/src/FlexLexer.h ; do
echo "-I$(dirname $d)" >> compile_flags.txt
done
-
-# clang-tidy sometimes has issues figuring out if a file is c++,
-# so let's tell it. Bant can't always exctract that yet from --config redirects
-# in .bazelrc
-echo "-xc++" >> compile_flags.txt
diff --git a/.github/bin/run-format.sh b/.github/bin/run-format.sh
index e6bedd6..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,15 +50,10 @@
(echo "-- Missing the clang-format binary! --"; exit 1)
fi
fi
+echo "Using ${CLANG_FORMAT} ($("${CLANG_FORMAT}" --version))"
BUILDIFIER=${BUILDIFIER:-buildifier}
-# Currently, we're using clang-format 17, as newer versions still have some
-# volatility in minor version.
-${CLANG_FORMAT} --version | grep "17\." ||
- ( echo "-- Need clang-format 17. Currently CLANG_FORMAT=$CLANG_FORMAT --";
- exit 1)
-
# Run on all files.
find . -name "*.h" -o -name "*.cc" \
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 ebe5679..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]=223
-ExpectedFailCount[preprocessor:ibex]=397
+ExpectedFailCount[project:ibex]=224
+ExpectedFailCount[preprocessor:ibex]=403
-ExpectedFailCount[syntax:opentitan]=92
-ExpectedFailCount[lint:opentitan]=92
-ExpectedFailCount[project:opentitan]=1179
+ExpectedFailCount[syntax:opentitan]=88
+ExpectedFailCount[lint:opentitan]=88
+ExpectedFailCount[project:opentitan]=1077
ExpectedFailCount[formatter:opentitan]=0
-ExpectedFailCount[preprocessor:opentitan]=3061
+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]=455
-ExpectedFailCount[preprocessor:caliptra-rtl]=903
+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
@@ -161,17 +145,17 @@
ExpectedFailCount[syntax:cva6]=7
ExpectedFailCount[lint:cva6]=7
-ExpectedFailCount[project:cva6]=92
-ExpectedFailCount[preprocessor:cva6]=141
+ExpectedFailCount[project:cva6]=84
+ExpectedFailCount[preprocessor:cva6]=140
-ExpectedFailCount[syntax:uvm]=0
-ExpectedFailCount[lint:uvm]=0
-ExpectedFailCount[project:uvm]=47
-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
-ExpectedFailCount[project:tnoc]=24
+ExpectedFailCount[project:tnoc]=21
ExpectedFailCount[preprocessor:tnoc]=57
ExpectedFailCount[project:80x86]=2
@@ -187,17 +171,17 @@
ExpectedFailCount[project:black-parrot]=172
ExpectedFailCount[preprocessor:black-parrot]=173
-ExpectedFailCount[syntax:ivtest]=166
-ExpectedFailCount[lint:ivtest]=166
-ExpectedFailCount[project:ivtest]=198
+ExpectedFailCount[syntax:ivtest]=116
+ExpectedFailCount[lint:ivtest]=116
+ExpectedFailCount[project:ivtest]=145
ExpectedFailCount[preprocessor:ivtest]=26
ExpectedFailCount[syntax:nontrivial-mips]=2
ExpectedFailCount[lint:nontrivial-mips]=2
-ExpectedFailCount[project:nontrivial-mips]=81
+ExpectedFailCount[project:nontrivial-mips]=79
ExpectedFailCount[preprocessor:nontrivial-mips]=78
-ExpectedFailCount[project:axi]=82
+ExpectedFailCount[project:axi]=79
ExpectedFailCount[preprocessor:axi]=79
ExpectedFailCount[syntax:rsd]=5
@@ -205,15 +189,15 @@
ExpectedFailCount[project:rsd]=52
ExpectedFailCount[preprocessor:rsd]=49
-ExpectedFailCount[project:scr1]=45
+ExpectedFailCount[project:scr1]=44
ExpectedFailCount[preprocessor:scr1]=46
ExpectedFailCount[project:serv]=1
ExpectedFailCount[preprocessor:serv]=1
-ExpectedFailCount[syntax:basejump_stl]=499
-ExpectedFailCount[lint:basejump_stl]=499
-ExpectedFailCount[project:basejump_stl]=620
+ExpectedFailCount[syntax:basejump_stl]=498
+ExpectedFailCount[lint:basejump_stl]=498
+ExpectedFailCount[project:basejump_stl]=619
ExpectedFailCount[formatter:basejump_stl]=1
ExpectedFailCount[preprocessor:basejump_stl]=659
@@ -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/settings.sh b/.github/settings.sh
index a6d4ae8..9f56dee 100644
--- a/.github/settings.sh
+++ b/.github/settings.sh
@@ -29,7 +29,7 @@
export GIT_VERSION=${GIT_VERSION:-$(git describe --match='v*')}
-export BAZEL_CXXOPTS="-std=c++17"
+export BAZEL_CXXOPTS="-std=c++20"
# Progress output is just noisy in CI outputs.
export BAZEL_OPTS="-c opt --noshow_progress"
diff --git a/.github/workflows/verible-ci.yml b/.github/workflows/verible-ci.yml
index c1fd2ab..802a7a3 100644
--- a/.github/workflows/verible-ci.yml
+++ b/.github/workflows/verible-ci.yml
@@ -26,7 +26,7 @@
CheckFormatAndBuildClean:
- runs-on: ubuntu-24.04
+ runs-on: ubuntu-latest
steps:
- name: Checkout code
@@ -34,15 +34,22 @@
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
+ clang-format-19 --version
go install github.com/bazelbuild/buildtools/buildifier@latest
echo "PATH=$PATH:$(go env GOPATH)/bin/" >> $GITHUB_ENV
- name: Run formatting style check
run: |
echo "--- check formatting ---"
- CLANG_FORMAT=clang-format-17 ./.github/bin/run-format.sh --show-diff
+ CLANG_FORMAT=clang-format-19 ./.github/bin/run-format.sh --show-diff
echo "--- check build dependencies ---"
./.github/bin/run-build-cleaner.sh
echo "--- check potential problems ---"
@@ -124,7 +131,6 @@
- test
- test-clang
- test-nortti
- - test-c++20
- test-c++23
- smoke-test
#- smoke-test-analyzer #issue: #2046
@@ -141,8 +147,6 @@
exclude:
- mode: test-nortti
arch: arm64
- - mode: test-c++20
- arch: arm64
- mode: test-c++23
arch: arm64
- mode: asan
@@ -182,12 +186,10 @@
set -x
apt -qqy update
apt -qq -y install build-essential wget git python3 python-is-python3 default-jdk cmake python3-pip ripgrep
- apt -qq -y install gcc-10 g++-10
apt -qq -y install gcc-14 g++-14
apt -qq -y install clang-19
source ./.github/settings.sh
- # Use newer compiler for c++2x compilation. Also slang needs c++20
- ./.github/bin/set-compiler.sh $([[ "$MODE" == test-c++2* || "$MODE" == "smoke-test-analyzer" ]] && echo 14 || echo 10)
+ ./.github/bin/set-compiler.sh 14
ARCH="$ARCH" ./.github/bin/install-bazel.sh
- name: Build Slang
@@ -333,8 +335,8 @@
with:
path: |
/private/var/tmp/_bazel_runner
- key: bazelcache_macos_${{ steps.cache_timestamp.outputs.time }}
- restore-keys: bazelcache_macos_
+ key: bazelcache_macos1_${{ steps.cache_timestamp.outputs.time }}
+ restore-keys: bazelcache_macos1_
- name: Tests
# MacOS has a broken patch utility:
@@ -396,22 +398,17 @@
- name: Install dependencies
run: |
- choco install bazel --force --version=7.6.1
choco install winflexbison3
- choco install llvm --allow-downgrade --version=20.1.4
- name: Debug bazel directory settings
- # We need to explicitly call the bazel binary from choco, otherwise
- # the default Windows runner seems to run bazelisk(?) and downloads the
- # latest bazel, which is incompatible. Should be in variable.
- run: C:/ProgramData/chocolatey/lib/bazel/bazel.exe info
+ run: bazel.exe info
- name: Run Tests
- run: C:/ProgramData/chocolatey/lib/bazel/bazel.exe test --keep_going --noshow_progress --test_output=errors //...
+ run: bazel.exe test --keep_going --noshow_progress --test_output=errors //...
- name: Build Verible Binaries
run: |
- C:/ProgramData/chocolatey/lib/bazel/bazel.exe build --keep_going --noshow_progress -c opt :install-binaries
+ bazel.exe build --keep_going --noshow_progress -c opt :install-binaries
# Litmus test
bazel-bin/verible/verilog/tools/syntax/verible-verilog-syntax --version
@@ -432,7 +429,7 @@
# prevents packaging up the cache.
- name: Stop Bazel
run: |
- C:/ProgramData/chocolatey/lib/bazel/bazel.exe shutdown
+ bazel.exe shutdown
# The cache pack/restore has issues with these symbolic links
rm c:/users/runneradmin/_bazel_runneradmin/*/install
rm c:/users/runneradmin/_bazel_runneradmin/*/java.log
diff --git a/MODULE.bazel b/MODULE.bazel
index 998aa87..463c634 100644
--- a/MODULE.bazel
+++ b/MODULE.bazel
@@ -20,4 +20,4 @@
bazel_dep(name = "googletest", version = "1.17.0.bcr.2", dev_dependency = True)
# To build compilation DB and run build-cleaning
-bazel_dep(name = "bant", version = "0.2.10", dev_dependency = True)
+bazel_dep(name = "bant", version = "0.3.4", dev_dependency = True)
diff --git a/README.md b/README.md
index a139080..047fb20 100644
--- a/README.md
+++ b/README.md
@@ -172,10 +172,9 @@
Verible's code base is written in C++.
-To build, you need the [bazel] build system and a C++17
-compatible compiler (e.g. >= g++-10), as well as python3.
-A lot of users of Verible have to work on pretty old installations,
-so we try to keep the requirements as minimal as possible.
+To build, you need the [bazel] build system (get it from
+[bazel install][bazel-install] if not already on your system) and a
+C++20 compatible compiler.
Use your package manager to install the dependencies; on a system with
the nix package manager simply run `nix-shell` to get a build environment.
@@ -201,6 +200,8 @@
bazel build -c opt --config=create_static_linked_executables //...
```
+See [Installation](#installation-1) for install.
+
### Optionally using local flex/bison for build
Flex and Bison, that are needed for the parser generation, are compiled as part
@@ -216,17 +217,16 @@
### Building on Windows
-Building on Windows requires LLVM, WinFlexBison 3 and Git-bash to be installed. Using package manager [chocolatey], this can be done with
+In addition to Bazel & Visual Studio, building on Windows requires WinFlexBison 3 and Git-bash to be installed. Using package manager [chocolatey], this can be done with
```powershell
-choco install git llvm winflexbison3
+choco install git winflexbison3
```
-Bazel may also require environment variable to use git-bash and LLVM, on powershell
+Bazel may also require environment variable to use git-bash, on powershell
```powershell
$env:BAZEL_SH="C:\Program Files\Git\git-bash.exe"
-$env:BAZEL_LLVM="C:\Program Files\LLVM"
```
### Installation
@@ -286,6 +286,7 @@
[UHDM] format. If you are interested in collaborating, contact us.
[bazel]: https://bazel.build/
+[bazel-install]: https://bazel.build/install
[SV-LRM]: https://ieeexplore.ieee.org/document/8299595
[lint-rule-list]: https://chipsalliance.github.io/verible/lint.html
[github-lint-action]: https://github.com/chipsalliance/verible-linter-action
diff --git a/bazel/BUILD b/bazel/BUILD
index 6dab72b..ebe6d5b 100644
--- a/bazel/BUILD
+++ b/bazel/BUILD
@@ -4,6 +4,7 @@
load("@bazel_skylib//rules:common_settings.bzl", "bool_flag")
load("@rules_cc//cc:defs.bzl", "cc_library")
+load(":no_toolchain.bzl", "no_toolchain")
package(
default_applicable_licenses = ["//:license"],
@@ -23,6 +24,7 @@
exports_files([
"bison.bzl",
"flex.bzl",
+ "no_toolchain.bzl",
"sh_test_with_runfiles_lib.bzl",
"sh_test_with_runfiles_lib.sh",
])
@@ -37,6 +39,12 @@
flag_values = {":use_local_flex_bison": "true"},
)
+# Placeholder used where a select() would otherwise need to switch a
+# genrule's non-configurable `toolchains` attribute (see flex.bzl/bison.bzl):
+no_toolchain(
+ name = "no_toolchain",
+)
+
bool_flag(
name = "create_static_linked_executables",
build_setting_default = False,
diff --git a/bazel/bison.bzl b/bazel/bison.bzl
index f191209..9053b03 100644
--- a/bazel/bison.bzl
+++ b/bazel/bison.bzl
@@ -26,6 +26,23 @@
extra_outs = []):
"""Build rule for generating C or C++ sources with Bison.
"""
+
+ native.alias(
+ name = name + "_bison_toolchain",
+ actual = select({
+ "//bazel:use_local_flex_bison_enabled": "//bazel:no_toolchain",
+ "@platforms//os:windows": "//bazel:no_toolchain",
+ "//conditions:default": "@rules_bison//bison:current_bison_toolchain",
+ }),
+ )
+ native.alias(
+ name = name + "_m4_toolchain",
+ actual = select({
+ "//bazel:use_local_flex_bison_enabled": "//bazel:no_toolchain",
+ "@platforms//os:windows": "//bazel:no_toolchain",
+ "//conditions:default": "@rules_m4//m4:current_m4_toolchain",
+ }),
+ )
native.genrule(
name = name,
srcs = [src],
@@ -35,12 +52,8 @@
"@platforms//os:windows": "win_bison.exe --defines=$(location " + header_out + ") --output-file=$(location " + source_out + ") " + " ".join(extra_options) + " $<",
"//conditions:default": "M4=$(M4) $(BISON) --defines=$(location " + header_out + ") --output-file=$(location " + source_out + ") " + " ".join(extra_options) + " $<",
}),
- toolchains = select({
- "//bazel:use_local_flex_bison_enabled": [],
- "@platforms//os:windows": [],
- "//conditions:default": [
- "@rules_bison//bison:current_bison_toolchain",
- "@rules_m4//m4:current_m4_toolchain",
- ],
- }),
+ toolchains = [
+ ":" + name + "_bison_toolchain",
+ ":" + name + "_m4_toolchain",
+ ],
)
diff --git a/bazel/flex.bzl b/bazel/flex.bzl
index a398c6f..98f6c8c 100644
--- a/bazel/flex.bzl
+++ b/bazel/flex.bzl
@@ -20,6 +20,23 @@
def genlex(name, src, out):
"""Generate C/C++ language source from lex file using Flex
"""
+
+ native.alias(
+ name = name + "_flex_toolchain",
+ actual = select({
+ "//bazel:use_local_flex_bison_enabled": "//bazel:no_toolchain",
+ "@platforms//os:windows": "//bazel:no_toolchain",
+ "//conditions:default": "@rules_flex//flex:current_flex_toolchain",
+ }),
+ )
+ native.alias(
+ name = name + "_m4_toolchain",
+ actual = select({
+ "//bazel:use_local_flex_bison_enabled": "//bazel:no_toolchain",
+ "@platforms//os:windows": "//bazel:no_toolchain",
+ "//conditions:default": "@rules_m4//m4:current_m4_toolchain",
+ }),
+ )
native.genrule(
name = name,
srcs = [src],
@@ -29,12 +46,8 @@
"@platforms//os:windows": "win_flex.exe --outfile=$@ $<",
"//conditions:default": "M4=$(M4) $(FLEX) --outfile=$@ $<",
}),
- toolchains = select({
- "//bazel:use_local_flex_bison_enabled": [],
- "@platforms//os:windows": [],
- "//conditions:default": [
- "@rules_flex//flex:current_flex_toolchain",
- "@rules_m4//m4:current_m4_toolchain",
- ],
- }),
+ toolchains = [
+ ":" + name + "_flex_toolchain",
+ ":" + name + "_m4_toolchain",
+ ],
)
diff --git a/bazel/no_toolchain.bzl b/bazel/no_toolchain.bzl
new file mode 100644
index 0000000..07d4225
--- /dev/null
+++ b/bazel/no_toolchain.bzl
@@ -0,0 +1,28 @@
+# -*- Python -*-
+# Copyright 2017-2026 The Verible Authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""A placeholder target for genrule's `toolchains` attribute.
+
+genrule's `toolchains` attribute requires each entry to provide
+TemplateVariableInfo (or ToolchainTypeInfo). Used where a platform (e.g.
+Windows) needs no real toolchain there, like for using local winflexbison.
+"""
+
+def _no_toolchain_impl(ctx):
+ return [platform_common.TemplateVariableInfo({})]
+
+no_toolchain = rule(
+ implementation = _no_toolchain_impl,
+)
diff --git a/external_libs/editscript.h b/external_libs/editscript.h
index 8e01ef9..1f4dcb8 100644
--- a/external_libs/editscript.h
+++ b/external_libs/editscript.h
@@ -179,8 +179,10 @@
template <typename TokenIter>
class Diff {
private:
- friend Edits GetTokenDiffs<>(TokenIter tokens1_begin, TokenIter tokens1_end,
- TokenIter tokens2_begin, TokenIter tokens2_end);
+ friend Edits diff::GetTokenDiffs<>(TokenIter tokens1_begin,
+ TokenIter tokens1_end,
+ TokenIter tokens2_begin,
+ TokenIter tokens2_end);
/**
* Finds the differences between two vectors of tokens, returning edits
diff --git a/shell.nix b/shell.nix
index 7a8f55e..85163eb 100644
--- a/shell.nix
+++ b/shell.nix
@@ -7,12 +7,13 @@
verible_used_stdenv = pkgs.stdenv;
#verible_used_stdenv = pkgs.gcc15Stdenv;
#verible_used_stdenv = pkgs.clang19Stdenv;
+ bazel = pkgs.bazel_8;
in
verible_used_stdenv.mkDerivation {
name = "verible-build-environment";
buildInputs = with pkgs;
[
- bazel_7
+ bazel
git
# For scripts used inside bzl rules and tests
@@ -37,14 +38,14 @@
lcov # coverage html generation.
bazel-buildtools # buildifier
- llvmPackages_21.clang-tools # for clang-tidy
- llvmPackages_18.clang-tools # for clang-format
+ llvmPackages_22.clang-tools # for clang-tidy
+ llvmPackages_19.clang-tools # for clang-format
];
shellHook = ''
# clang tidy: use latest.
- export CLANG_TIDY=${pkgs.llvmPackages_21.clang-tools}/bin/clang-tidy
+ export CLANG_TIDY=${pkgs.llvmPackages_22.clang-tools}/bin/clang-tidy
# Last version that current github CI supports.
- export CLANG_FORMAT=${pkgs.llvmPackages_18.clang-tools}/bin/clang-format
+ export CLANG_FORMAT=${pkgs.llvmPackages_19.clang-tools}/bin/clang-format
'';
}
diff --git a/verible/common/formatting/align.cc b/verible/common/formatting/align.cc
index 9986fc7..74bc432 100644
--- a/verible/common/formatting/align.cc
+++ b/verible/common/formatting/align.cc
@@ -1373,7 +1373,7 @@
break;
}
} // switch
- } // for
+ } // for
// Flush out the last range.
if (match_count >= min_match_count) {
result.emplace_back(last_range_start, partitions.end(), last_match_subtype);
diff --git a/verible/common/formatting/align.h b/verible/common/formatting/align.h
index d042ae0..f290ac6 100644
--- a/verible/common/formatting/align.h
+++ b/verible/common/formatting/align.h
@@ -407,22 +407,33 @@
// Skip tree tokens. Non-tree tokens located between tree tokens (e.g. block
// comments) are also skipped.
while (*(ftoken_it->token) != last_tree_token) ++ftoken_it;
- // Use next token as begining of trailing non-tree tokens
+ // Use next token as beginning of trailing non-tree tokens
trailing_tokens.set_begin(ftoken_it + 1);
- // Breaking following condition leads to e.g. concatenation of EOL comment
- // and code in a single line. To fix situation that lead to this, flatten
- // token partitions that contain EOL comment subpartition just before a
- // subpartition that starts with the same token as Origin(). Example of a
- // partition that needs flattening:
+ // When leading non-tree tokens (typically a comment block between list
+ // items) coexist with the first tree token being forced to start on a
+ // new line, the leading tokens belong on the preceding line and the
+ // tree content on a separate line. Discard the leading tokens from
+ // alignment consideration instead of trying to align them.
+ // (This can also arise from EOL comments in a partition that needs
+ // flattening; see the example below.)
//
+ // Example of a partition tree that needs flattening (from the tree
+ // unwrapper, not this code):
// { (>>[...], (origin: "input bit second"))
// { (>>[// comment] }
// { (>>[input bit second], (origin: "input")) }
// }
- CHECK(leading_tokens.empty() || first_tree_token_it == ftokens.end() ||
- first_tree_token_it->before.break_decision !=
- SpacingOptions::kMustWrap);
+ if (!leading_tokens.empty() && first_tree_token_it != ftokens.end() &&
+ first_tree_token_it->before.break_decision ==
+ SpacingOptions::kMustWrap) {
+ // The partition has comment tokens that were absorbed into the
+ // following item's partition (a tree-unwrapper flattening gap).
+ // Return tree-scanned columns without non-tree additions,
+ // effectively skipping alignment for this row rather than
+ // risking corruption of the layout.
+ return column_entries;
+ }
} else {
// All tokens are passed as leading
leading_tokens.set_end(ftokens.end());
diff --git a/verible/common/formatting/align_test.cc b/verible/common/formatting/align_test.cc
index d667697..b9bdb47 100644
--- a/verible/common/formatting/align_test.cc
+++ b/verible/common/formatting/align_test.cc
@@ -740,11 +740,12 @@
partition_.Children().end());
const std::vector<TaggedTokenPartitionRange> ranges(
- GetPartitionAlignmentSubranges(children, [](const TokenPartitionTree
- &partition) {
- // Don't care about the subtype tag.
- return AlignedPartitionClassification{PartitionSelector(partition), 0};
- }));
+ GetPartitionAlignmentSubranges(children,
+ [](const TokenPartitionTree &partition) {
+ // Don't care about the subtype tag.
+ return AlignedPartitionClassification{
+ PartitionSelector(partition), 0};
+ }));
using P = std::pair<int, int>;
std::vector<P> range_indices;
diff --git a/verible/common/strings/patch.cc b/verible/common/strings/patch.cc
index 966307b..aced6a0 100644
--- a/verible/common/strings/patch.cc
+++ b/verible/common/strings/patch.cc
@@ -229,7 +229,8 @@
int line_number = header_.old_range.start; // 1-indexed
for (const MarkedLine &line : lines_) {
if (line.IsAdded()) continue; // ignore added lines
- if (line_number > static_cast<int>(original_lines.size())) {
+ if (line_number < 1 ||
+ line_number > static_cast<int>(original_lines.size())) {
return absl::OutOfRangeError(absl::StrCat(
"Patch hunk references line ", line_number, " in a file with only ",
original_lines.size(), " lines"));
diff --git a/verible/common/strings/patch_test.cc b/verible/common/strings/patch_test.cc
index 17a4930..a3c9ae3 100644
--- a/verible/common/strings/patch_test.cc
+++ b/verible/common/strings/patch_test.cc
@@ -755,6 +755,34 @@
}
}
+TEST(HunkVerifyAgainstOriginalLinesTest, NonPositiveStartLine) {
+ // A hunk header whose old-file start is 0 (line numbers are 1-indexed) must
+ // be rejected rather than indexing original_lines at a non-positive offset.
+ const std::vector<std::string_view> kHunkText = {
+ {
+ "@@ -0,2 +1,2 @@", //
+ " line1", // retained line reached with line_number == 0
+ "-line2", //
+ "+line pi", //
+ },
+ };
+ const std::vector<std::string_view> kOriginal = {
+ "line1",
+ "line2",
+ };
+ Hunk hunk;
+ {
+ const LineRange range(kHunkText.begin(), kHunkText.end());
+ const auto status = hunk.Parse(range);
+ ASSERT_TRUE(status.ok()) << status.message();
+ }
+ {
+ const auto status = hunk.VerifyAgainstOriginalLines(kOriginal);
+ EXPECT_EQ(status.code(), absl::StatusCode::kOutOfRange);
+ EXPECT_TRUE(absl::StrContains(status.message(), "references line 0"));
+ }
+}
+
TEST(HunkVerifyAgainstOriginalLinesTest, InconsistentRetainedLine) {
const std::vector<std::string_view> kHunkText = {
{
diff --git a/verible/common/text/parser-verifier.h b/verible/common/text/parser-verifier.h
index 08edcb9..90aa0bc 100644
--- a/verible/common/text/parser-verifier.h
+++ b/verible/common/text/parser-verifier.h
@@ -58,7 +58,7 @@
// TODO(jeremycs): changed these to protected and make SyntaxTreeLeaf
// and SyntaxTreeNode friend classes
void Visit(const SyntaxTreeLeaf &leaf) final;
- void Visit(const SyntaxTreeNode &node) final{};
+ void Visit(const SyntaxTreeNode &node) final {};
private:
const Symbol &root_;
diff --git a/verible/common/text/tree-utils_test.cc b/verible/common/text/tree-utils_test.cc
index ace2284..0a77530 100644
--- a/verible/common/text/tree-utils_test.cc
+++ b/verible/common/text/tree-utils_test.cc
@@ -810,13 +810,11 @@
// FindSubtreeStartingAtOffset tests
constexpr std::string_view kFindSubtreeTestText("abcdef");
-const std::string_view kFindSubtreeTestSubstring(
- kFindSubtreeTestText.substr(1, 3));
struct FindSubtreeStartingAtOffsetTest : public testing::Test {
SymbolPtr tree;
FindSubtreeStartingAtOffsetTest()
- : tree(Leaf(0, kFindSubtreeTestSubstring)) {}
+ : tree(Leaf(0, kFindSubtreeTestText.substr(1, 3))) {}
};
// Test that a single leaf yields itself when it starts < offset.
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/common/util/container-proxy_test.cc b/verible/common/util/container-proxy_test.cc
index 5e8220f..4e4cd75 100644
--- a/verible/common/util/container-proxy_test.cc
+++ b/verible/common/util/container-proxy_test.cc
@@ -874,13 +874,14 @@
const int new_capacity = initial_capacity + 42;
this->proxy.reserve(new_capacity);
- EXPECT_EQ(this->proxy.capacity(), new_capacity);
- EXPECT_EQ(this->container.capacity(), new_capacity);
+ EXPECT_GE(this->proxy.capacity(), new_capacity);
+ EXPECT_EQ(this->proxy.capacity(), this->container.capacity());
+ const int capacity_after_first_reserve = this->proxy.capacity();
const int lower_capacity = 1;
this->proxy.reserve(lower_capacity);
- EXPECT_EQ(this->proxy.capacity(), new_capacity);
- EXPECT_EQ(this->container.capacity(), new_capacity);
+ EXPECT_EQ(this->proxy.capacity(), capacity_after_first_reserve);
+ EXPECT_EQ(this->container.capacity(), capacity_after_first_reserve);
}
} // namespace
diff --git a/verible/common/util/tree-operations.h b/verible/common/util/tree-operations.h
index b70da51..231d767 100644
--- a/verible/common/util/tree-operations.h
+++ b/verible/common/util/tree-operations.h
@@ -174,9 +174,11 @@
// `TreeNodeTraits<T>` is defined for every class `T` fulfilling the TreeNode
// concept. It can be used in SFINAE tests.
template <class Node, //
- typename Children_ =
- tree_operations_internal::TreeNodeChildrenTraits<Node>>
+ typename Children_ = detected_or_t<
+ UnavailableFeatureTraits,
+ tree_operations_internal::TreeNodeChildrenTraits, Node>>
struct TreeNodeTraits : FeatureTraits {
+ static constexpr bool available = Children_::available;
using Parent =
detected_or_t<UnavailableFeatureTraits,
tree_operations_internal::TreeNodeParentTraits, Node>;
diff --git a/verible/verilog/CST/BUILD b/verible/verilog/CST/BUILD
index 39207e0..03aa2bf 100644
--- a/verible/verilog/CST/BUILD
+++ b/verible/verilog/CST/BUILD
@@ -749,6 +749,8 @@
"//verible/common/text:symbol",
"//verible/common/text:tree-utils",
"//verible/common/text:visitors",
+ "//verible/common/util:casts",
+ "//verible/verilog/parser:verilog-token-enum",
],
)
diff --git a/verible/verilog/CST/statement.cc b/verible/verilog/CST/statement.cc
index 0dce48c..b3923bd 100644
--- a/verible/verilog/CST/statement.cc
+++ b/verible/verilog/CST/statement.cc
@@ -21,11 +21,13 @@
#include "verible/common/text/concrete-syntax-tree.h"
#include "verible/common/text/symbol.h"
#include "verible/common/text/tree-utils.h"
+#include "verible/common/util/casts.h"
#include "verible/verilog/CST/declaration.h"
#include "verible/verilog/CST/identifier.h"
#include "verible/verilog/CST/type.h"
#include "verible/verilog/CST/verilog-matchers.h" // IWYU pragma: keep
#include "verible/verilog/CST/verilog-nonterminals.h"
+#include "verible/verilog/parser/verilog-token-enum.h"
namespace verilog {
@@ -447,6 +449,21 @@
}
// Returns the data type node from for loop initialization.
+const verible::SyntaxTreeLeaf *GetGenvarKeywordFromForInitialization(
+ const verible::Symbol &for_initialization) {
+ const verible::Symbol *child0 =
+ GetSubtreeAsSymbol(for_initialization, NodeEnum::kForInitialization, 0);
+ if (child0 == nullptr) return nullptr;
+ if (child0->Kind() == verible::SymbolKind::kLeaf) {
+ const auto *leaf =
+ verible::down_cast<const verible::SyntaxTreeLeaf *>(child0);
+ if (leaf->get().token_enum() == TK_genvar) {
+ return leaf;
+ }
+ }
+ return nullptr;
+}
+
const verible::SyntaxTreeNode *GetDataTypeFromForInitialization(
const verible::Symbol &for_initialization) {
const auto *data_type = verible::GetSubtreeAsSymbol(
diff --git a/verible/verilog/CST/statement.h b/verible/verilog/CST/statement.h
index 2b8ba39..27fe670 100644
--- a/verible/verilog/CST/statement.h
+++ b/verible/verilog/CST/statement.h
@@ -179,8 +179,13 @@
const verible::Symbol &conditional);
// Returns the data type node from for loop initialization.
+// Returns the genvar keyword leaf from a for initialization, or nullptr if not
+// present.
+const verible::SyntaxTreeLeaf *GetGenvarKeywordFromForInitialization(
+ const verible::Symbol &for_initialization);
+
const verible::SyntaxTreeNode *GetDataTypeFromForInitialization(
- const verible::Symbol &);
+ const verible::Symbol &for_initialization);
// Returns the variable name leaf from for loop initialization.
const verible::SyntaxTreeLeaf *GetVariableNameFromForInitialization(
diff --git a/verible/verilog/CST/verilog-matchers.h b/verible/verilog/CST/verilog-matchers.h
index bc41a60..6663e4a 100644
--- a/verible/verilog/CST/verilog-matchers.h
+++ b/verible/verilog/CST/verilog-matchers.h
@@ -437,6 +437,8 @@
// ...
inline constexpr auto HasUniqueQualifier =
verible::matcher::MakePathMatcher(L(TK_unique));
+inline constexpr auto HasUnique0Qualifier =
+ verible::matcher::MakePathMatcher(L(TK_unique0));
// Clean up macros
#undef N
#undef L
diff --git a/verible/verilog/CST/verilog-matchers_test.cc b/verible/verilog/CST/verilog-matchers_test.cc
index c38edff..bb0a4c9 100644
--- a/verible/verilog/CST/verilog-matchers_test.cc
+++ b/verible/verilog/CST/verilog-matchers_test.cc
@@ -822,10 +822,69 @@
endfunction
)",
1},
+ {HasUniqueQualifier(),
+ R"(
+ function automatic int foo (input in);
+ unique0 case (in)
+ 1: return 0;
+ endcase
+ endfunction
+ )",
+ 0},
};
for (const auto &test : tests) {
verible::matcher::RunRawMatcherTestCase<VerilogAnalyzer>(test);
}
}
+
+// Tests for HasUnique0Qualifier matching.
+TEST(VerilogMatchers, HasUnique0QualifierTests) {
+ const RawMatcherTestCase tests[] = {
+ {HasUnique0Qualifier(), "", 0},
+ {HasUnique0Qualifier(),
+ R"(
+ function automatic int foo (input in);
+ case (in)
+ default: return 0;
+ endcase
+ endfunction
+ )",
+ 0},
+ {HasUnique0Qualifier(),
+ R"(
+ function automatic int foo (input in);
+ unique0 case (in)
+ 1: return 0;
+ endcase
+ endfunction
+ )",
+ 1},
+ {HasUnique0Qualifier(),
+ R"(
+ function automatic int foo (input in);
+ unique case (in)
+ 1: return 0;
+ endcase
+ endfunction
+ )",
+ 0},
+ {HasUnique0Qualifier(),
+ R"(
+ function automatic int foo (input in);
+ unique0 if (in) begin
+ return 0;
+ end
+ else if(!in) begin
+ return 1;
+ end
+ endfunction
+ )",
+ 1},
+ };
+ for (const auto &test : tests) {
+ verible::matcher::RunRawMatcherTestCase<VerilogAnalyzer>(test);
+ }
+}
+
} // namespace
} // namespace verilog
diff --git a/verible/verilog/analysis/checkers/BUILD b/verible/verilog/analysis/checkers/BUILD
index a83f76a..4681607 100644
--- a/verible/verilog/analysis/checkers/BUILD
+++ b/verible/verilog/analysis/checkers/BUILD
@@ -817,6 +817,7 @@
"//verible/common/analysis:syntax-tree-lint-rule",
"//verible/common/analysis/matcher",
"//verible/common/analysis/matcher:bound-symbol-manager",
+ "//verible/common/text:config-utils",
"//verible/common/text:symbol",
"//verible/common/text:syntax-tree-context",
"//verible/common/text:token-info",
@@ -826,7 +827,9 @@
"//verible/verilog/CST:verilog-nonterminals",
"//verible/verilog/analysis:descriptions",
"//verible/verilog/analysis:lint-rule-registry",
+ "@abseil-cpp//absl/status",
"@abseil-cpp//absl/strings",
+ "@re2",
],
alwayslink = 1,
)
diff --git a/verible/verilog/analysis/checkers/case-missing-default-rule.cc b/verible/verilog/analysis/checkers/case-missing-default-rule.cc
index c401cd6..8f8ec39 100644
--- a/verible/verilog/analysis/checkers/case-missing-default-rule.cc
+++ b/verible/verilog/analysis/checkers/case-missing-default-rule.cc
@@ -42,7 +42,7 @@
static constexpr std::string_view kMessage =
"Explicitly define a default case for every case statement or add `unique` "
- "qualifier to the case statement.";
+ "or `unique0` qualifier to the case statement.";
const LintRuleDescriptor &CaseMissingDefaultRule::GetDescriptor() {
static const LintRuleDescriptor d{
@@ -50,7 +50,7 @@
.topic = "case-statements",
.desc =
"Checks that a default case-item is always defined unless the case "
- "statement has the `unique` qualifier.",
+ "statement has the `unique` or `unique0` qualifier.",
};
return d;
}
@@ -67,12 +67,16 @@
static const Matcher uniqueCaseMatcher(
NodekCaseStatement(HasUniqueQualifier()));
+ static const Matcher unique0CaseMatcher(
+ NodekCaseStatement(HasUnique0Qualifier()));
+
static const Matcher caseMatcherWithDefaultCase(
NodekCaseStatement(HasDefaultCase()));
// If the case statement doesn't have the "unique" qualifier and
// it is missing the "default" case, insert the violation
if (!uniqueCaseMatcher.Matches(symbol, &manager) &&
+ !unique0CaseMatcher.Matches(symbol, &manager) &&
!caseMatcherWithDefaultCase.Matches(symbol, &manager)) {
violations_.insert(LintViolation(symbol, kMessage, context));
}
diff --git a/verible/verilog/analysis/checkers/case-missing-default-rule_test.cc b/verible/verilog/analysis/checkers/case-missing-default-rule_test.cc
index 2b465df..84b8e77 100644
--- a/verible/verilog/analysis/checkers/case-missing-default-rule_test.cc
+++ b/verible/verilog/analysis/checkers/case-missing-default-rule_test.cc
@@ -52,6 +52,22 @@
endfunction
)"},
{R"(
+ function automatic int foo (input [2:0] in);
+ unique case (in)
+ 3'b001: return 1;
+ endcase
+ return 0;
+ endfunction
+ )"},
+ {R"(
+ function automatic int foo (input [2:0] in);
+ unique0 case (in)
+ 3'b001: return 1;
+ endcase
+ return 0;
+ endfunction
+ )"},
+ {R"(
function automatic int foo (input in);
)",
{TK_case, "case"},
@@ -70,6 +86,22 @@
endfunction
)"},
{R"(
+ function automatic int foo (input [2:0] in);
+ unique casex (in)
+ 3'b001: return 1;
+ endcase
+ return 0;
+ endfunction
+ )"},
+ {R"(
+ function automatic int foo (input [2:0] in);
+ unique0 casex (in)
+ 3'b001: return 1;
+ endcase
+ return 0;
+ endfunction
+ )"},
+ {R"(
function automatic int foo (input in);
)",
{TK_casex, "casex"},
@@ -88,6 +120,22 @@
endfunction
)"},
{R"(
+ function automatic int foo (input [2:0] in);
+ unique casez (in)
+ 3'b001: return 1;
+ endcase
+ return 0;
+ endfunction
+ )"},
+ {R"(
+ function automatic int foo (input [2:0] in);
+ unique0 casez (in)
+ 3'b001: return 1;
+ endcase
+ return 0;
+ endfunction
+ )"},
+ {R"(
function automatic int foo (input in);
)",
{TK_casez, "casez"},
@@ -96,6 +144,20 @@
endcase
endfunction
)"},
+ // A qualified outer case does not exempt an unqualified inner case.
+ {R"(
+ function automatic int foo (input in);
+ unique0 case (in)
+ 1: begin;
+ )",
+ {TK_case, "case"},
+ R"( (in)
+ 1: return 1;
+ endcase
+ end
+ endcase
+ endfunction
+ )"},
// randcase should not be flagged
{R"(
diff --git a/verible/verilog/analysis/checkers/forbid-consecutive-null-statements-rule_test.cc b/verible/verilog/analysis/checkers/forbid-consecutive-null-statements-rule_test.cc
index 83914c6..eb483eb 100644
--- a/verible/verilog/analysis/checkers/forbid-consecutive-null-statements-rule_test.cc
+++ b/verible/verilog/analysis/checkers/forbid-consecutive-null-statements-rule_test.cc
@@ -226,12 +226,12 @@
TEST(ForbidConsecutiveNullStatementsRuleTest, ApplyAutoFix) {
const std::initializer_list<verible::AutoFixInOut> kTestCases = {
- {"module m;\ninitial begin ;; end\nendmodule",
- "module m;\ninitial begin ; end\nendmodule"},
- {"module m;\ninitial begin ; ; end\nendmodule",
- "module m;\ninitial begin ; end\nendmodule"},
- {"module m;\ninitial begin ; /* */; end\nendmodule",
- "module m;\ninitial begin ; /* */ end\nendmodule"},
+ {"module m;\ninitial begin ;; end\nendmodule",
+ "module m;\ninitial begin ; end\nendmodule"},
+ {"module m;\ninitial begin ; ; end\nendmodule",
+ "module m;\ninitial begin ; end\nendmodule"},
+ {"module m;\ninitial begin ; /* */; end\nendmodule",
+ "module m;\ninitial begin ; /* */ end\nendmodule"},
#if 0
// TODO: apply multi-violation fixes in linter_test_utils.
{ "module m;\ninitial begin; ; ; ; end\nendmodule",
diff --git a/verible/verilog/analysis/checkers/generate-label-prefix-rule.cc b/verible/verilog/analysis/checkers/generate-label-prefix-rule.cc
index c8822ce..e678ad5 100644
--- a/verible/verilog/analysis/checkers/generate-label-prefix-rule.cc
+++ b/verible/verilog/analysis/checkers/generate-label-prefix-rule.cc
@@ -14,12 +14,17 @@
#include "verible/verilog/analysis/checkers/generate-label-prefix-rule.h"
+#include <memory>
+#include <string>
#include <string_view>
-#include "absl/strings/match.h"
+#include "absl/status/status.h"
+#include "absl/strings/str_cat.h"
+#include "re2/re2.h"
#include "verible/common/analysis/lint-rule-status.h"
#include "verible/common/analysis/matcher/bound-symbol-manager.h"
#include "verible/common/analysis/matcher/matcher.h"
+#include "verible/common/text/config-utils.h"
#include "verible/common/text/symbol.h"
#include "verible/common/text/syntax-tree-context.h"
#include "verible/common/text/token-info.h"
@@ -38,22 +43,36 @@
// Register the lint rule
VERILOG_REGISTER_LINT_RULE(GenerateLabelPrefixRule);
-static constexpr std::string_view kMessage =
- "All generate block labels must start with g_ or gen_";
+static constexpr std::string_view kDefaultStyleRegex = "(g_|gen_).*";
-// TODO(fangism): and be lower_snake_case?
-// TODO(fangism): generalize to a configurable pattern and
-// rename this class/rule to GenerateLabelNamingStyle?
+GenerateLabelPrefixRule::GenerateLabelPrefixRule()
+ : style_regex_(
+ std::make_unique<re2::RE2>(kDefaultStyleRegex, re2::RE2::Quiet)) {}
const LintRuleDescriptor &GenerateLabelPrefixRule::GetDescriptor() {
static const LintRuleDescriptor d{
.name = "generate-label-prefix",
.topic = "generate-constructs",
- .desc = "Checks that every generate block label starts with g_ or gen_.",
+ .desc =
+ "Checks that every generate block label matches the regex defined by "
+ "style_regex. The default regex requires labels to start with g_ or "
+ "gen_. Refer to https://github.com/chipsalliance/verible/tree/master/"
+ "verilog/tools/lint#readme for more detail on verible regex "
+ "patterns.",
+ // NOLINTNEXTLINE(misc-include-cleaner)
+ .param = {{"style_regex", std::string(kDefaultStyleRegex),
+ "A regex used to check generate label style."}},
};
return d;
}
+std::string GenerateLabelPrefixRule::CreateViolationMessage() const {
+ return absl::StrCat(
+ "Generate block label does not match the naming convention defined by "
+ "regex pattern: ",
+ style_regex_->pattern());
+}
+
// Matches begin statements
static const Matcher &BlockMatcher() {
static const Matcher matcher(NodekGenerateBlock());
@@ -84,15 +103,24 @@
}
if (label != nullptr) {
- if (!(absl::StartsWith(label->text(), "g_") ||
- absl::StartsWith(label->text(), "gen_"))) {
- violations_.insert(verible::LintViolation(*label, kMessage, context));
+ if (!RE2::FullMatch(label->text(), *style_regex_)) {
+ violations_.insert(verible::LintViolation(
+ *label, CreateViolationMessage(), context));
}
}
}
}
}
+// NOLINTNEXTLINE(misc-include-cleaner)
+absl::Status GenerateLabelPrefixRule::Configure(
+ std::string_view configuration) {
+ using verible::config::SetRegex;
+ absl::Status s = verible::ParseNameValues(
+ configuration, {{"style_regex", SetRegex(&style_regex_)}});
+ return s;
+}
+
verible::LintRuleStatus GenerateLabelPrefixRule::Report() const {
return verible::LintRuleStatus(violations_, GetDescriptor());
}
diff --git a/verible/verilog/analysis/checkers/generate-label-prefix-rule.h b/verible/verilog/analysis/checkers/generate-label-prefix-rule.h
index 9090cf3..e18df9c 100644
--- a/verible/verilog/analysis/checkers/generate-label-prefix-rule.h
+++ b/verible/verilog/analysis/checkers/generate-label-prefix-rule.h
@@ -15,8 +15,13 @@
#ifndef VERIBLE_VERILOG_ANALYSIS_CHECKERS_GENERATE_LABEL_PREFIX_RULE_H_
#define VERIBLE_VERILOG_ANALYSIS_CHECKERS_GENERATE_LABEL_PREFIX_RULE_H_
+#include <memory>
#include <set>
+#include <string>
+#include <string_view>
+#include "absl/status/status.h"
+#include "re2/re2.h"
#include "verible/common/analysis/lint-rule-status.h"
#include "verible/common/analysis/syntax-tree-lint-rule.h"
#include "verible/common/text/symbol.h"
@@ -27,20 +32,28 @@
namespace analysis {
// GenerateLabelPrefixRule checks that all generate block labels start
-// with g_ or gen_
+// with g_ or gen_ (configurable via style_regex)
class GenerateLabelPrefixRule : public verible::SyntaxTreeLintRule {
public:
using rule_type = verible::SyntaxTreeLintRule;
+ GenerateLabelPrefixRule();
+
static const LintRuleDescriptor &GetDescriptor();
+ std::string CreateViolationMessage() const;
+
void HandleSymbol(const verible::Symbol &symbol,
const verible::SyntaxTreeContext &context) final;
verible::LintRuleStatus Report() const final;
+ absl::Status Configure(std::string_view configuration) final;
+
private:
std::set<verible::LintViolation> violations_;
+
+ std::unique_ptr<re2::RE2> style_regex_;
};
} // namespace analysis
diff --git a/verible/verilog/analysis/checkers/generate-label-prefix-rule_test.cc b/verible/verilog/analysis/checkers/generate-label-prefix-rule_test.cc
index b46bfe4..cb43501 100644
--- a/verible/verilog/analysis/checkers/generate-label-prefix-rule_test.cc
+++ b/verible/verilog/analysis/checkers/generate-label-prefix-rule_test.cc
@@ -27,6 +27,7 @@
namespace {
using verible::LintTestCase;
+using verible::RunConfiguredLintTestCases;
using verible::RunLintTestCases;
TEST(GenerateLabelPrefixRuleTest, Various) {
@@ -229,6 +230,40 @@
RunLintTestCases<VerilogAnalyzer, GenerateLabelPrefixRule>(kTestCases);
}
+TEST(GenerateLabelPrefixRuleTest, CustomRegex) {
+ const std::initializer_list<LintTestCase> kTestCases = {
+ {"module m;\n"
+ "generate\n"
+ "if (1) begin : my_custom_label\n"
+ "end\n"
+ "endgenerate\nendmodule\n"},
+ {"module m;\n"
+ "generate\n"
+ "for (genvar i=0; i<5; i++) begin : my_custom_label\n"
+ "end\n"
+ "endgenerate\nendmodule\n"},
+ };
+ RunConfiguredLintTestCases<VerilogAnalyzer, GenerateLabelPrefixRule>(
+ kTestCases, "style_regex:my_.*");
+
+ const std::initializer_list<LintTestCase> kFailCases = {
+ {"module m;\n"
+ "generate\n"
+ "if (1) begin : ",
+ {SymbolIdentifier, "g_label"},
+ "\nend\n"
+ "endgenerate\nendmodule\n"},
+ {"module m;\n"
+ "generate\n"
+ "if (1) begin : ",
+ {SymbolIdentifier, "gen_label"},
+ "\nend\n"
+ "endgenerate\nendmodule\n"},
+ };
+ RunConfiguredLintTestCases<VerilogAnalyzer, GenerateLabelPrefixRule>(
+ kFailCases, "style_regex:my_.*");
+}
+
} // namespace
} // namespace analysis
} // namespace verilog
diff --git a/verible/verilog/analysis/flow-tree.h b/verible/verilog/analysis/flow-tree.h
index d099bae..33bafa0 100644
--- a/verible/verilog/analysis/flow-tree.h
+++ b/verible/verilog/analysis/flow-tree.h
@@ -77,7 +77,7 @@
using VariantReceiver = std::function<bool(const Variant &variant)>;
explicit FlowTree(verible::TokenSequence source_sequence)
- : source_sequence_(std::move(source_sequence)){};
+ : source_sequence_(std::move(source_sequence)) {};
// Generates all possible variants.
absl::Status GenerateVariants(const VariantReceiver &receiver);
diff --git a/verible/verilog/analysis/symbol-table.cc b/verible/verilog/analysis/symbol-table.cc
index 9847f46..7d51108 100644
--- a/verible/verilog/analysis/symbol-table.cc
+++ b/verible/verilog/analysis/symbol-table.cc
@@ -89,6 +89,9 @@
{"class", SymbolMetaType::kClass},
{"module", SymbolMetaType::kModule},
{"package", SymbolMetaType::kPackage},
+ {"generate", SymbolMetaType::kGenerate},
+ {"procedural block", SymbolMetaType::kProceduralBlock},
+ {"loop scope", SymbolMetaType::kLoopScope},
{"parameter", SymbolMetaType::kParameter},
{"typedef", SymbolMetaType::kTypeAlias},
{"data/net/var/instance", SymbolMetaType::kDataNetVariableInstance},
@@ -98,6 +101,8 @@
{"enum", SymbolMetaType::kEnumType},
{"<enum constant>", SymbolMetaType::kEnumConstant},
{"interface", SymbolMetaType::kInterface},
+ {"genvar", SymbolMetaType::kGenvar},
+ {"generate loop", SymbolMetaType::kGenerateLoop},
{"<unspecified>", SymbolMetaType::kUnspecified},
{"<callable>", SymbolMetaType::kCallable},
});
@@ -210,6 +215,18 @@
case NodeEnum::kGenerateElseClause:
DeclareGenerateElse(node);
break;
+
+ // separate cases for handling different parts of generate construct
+ case NodeEnum::kLoopGenerateConstruct:
+ DeclareGenerateLoop(node);
+ break;
+ case NodeEnum::kGenerateBlock:
+ DeclareGenerateBlock(node);
+ break;
+ case NodeEnum::kGenvarDeclaration:
+ DeclareGenvarDeclaration(node);
+ break;
+
case NodeEnum::kPackageDeclaration:
DeclarePackage(node);
break;
@@ -306,6 +323,18 @@
// TODO(#1241) Not handled right now.
// TODO(#1255) Not handled right now.
break;
+ // separate case for blocks
+ case NodeEnum::kSeqBlock:
+ DeclareSequentialBlock(node);
+ break;
+ // separate case for for-loops
+ case NodeEnum::kForLoopStatement:
+ DeclareForLoop(node);
+ break;
+ // separate case for for-loop initialization
+ case NodeEnum::kForInitialization:
+ DeclareForInitVariable(node);
+ break;
default:
Descend(node);
break;
@@ -1242,28 +1271,17 @@
SymbolMetaType::kModule);
}
- std::string_view GetScopeNameFromGenerateBody(const SyntaxTreeNode &body) {
- if (body.MatchesTag(NodeEnum::kGenerateBlock)) {
- const SyntaxTreeNode *gen_block = GetGenerateBlockBegin(body);
- const TokenInfo *label =
- gen_block ? GetBeginLabelTokenInfo(*gen_block) : nullptr;
- if (label != nullptr) {
- // TODO: Check for a matching end-label here, and if its name matches
- // the begin label, then immediately create a resolved reference because
- // it only makes sense for it resolve to this begin.
- // Otherwise, do nothing with the end label.
- return label->text();
- }
- }
- return current_scope_->Value().CreateAnonymousScope("generate");
- }
-
void DeclareGenerateIf(const SyntaxTreeNode &generate_if) {
const SyntaxTreeNode *body(GetIfClauseGenerateBody(generate_if));
if (body) {
- DeclareScopedElementAndDescend(generate_if,
- GetScopeNameFromGenerateBody(*body),
- SymbolMetaType::kGenerate);
+ if (body->MatchesTag(NodeEnum::kGenerateBlock)) {
+ Descend(generate_if);
+ } else {
+ DeclareScopedElementAndDescend(
+ generate_if,
+ current_scope_->Value().CreateAnonymousScope("generate"),
+ SymbolMetaType::kGenerate);
+ }
}
}
@@ -1276,13 +1294,79 @@
// and let the if-clause inside create a scope directly under the current
// scope.
Descend(*body);
+ } else if (body->MatchesTag(NodeEnum::kGenerateBlock)) {
+ Descend(generate_else);
} else {
- DeclareScopedElementAndDescend(generate_else,
- GetScopeNameFromGenerateBody(*body),
+ DeclareScopedElementAndDescend(
+ generate_else,
+ current_scope_->Value().CreateAnonymousScope("generate"),
+ SymbolMetaType::kGenerate);
+ }
+ }
+
+ void DeclareGenerateLoop(const SyntaxTreeNode &node) {
+ const std::string_view scope_name =
+ current_scope_->Value().CreateAnonymousScope("generate-for");
+ DeclareScopedElementAndDescend(node, scope_name,
+ SymbolMetaType::kGenerateLoop);
+ }
+
+ void DeclareGenerateBlock(const SyntaxTreeNode &node) {
+ const SyntaxTreeNode *gen_block = GetGenerateBlockBegin(node);
+ const TokenInfo *label =
+ gen_block ? GetBeginLabelTokenInfo(*gen_block) : nullptr;
+ if (label != nullptr) {
+ DeclareScopedElementAndDescend(node, label->text(),
+ SymbolMetaType::kGenerate);
+ } else {
+ const std::string_view scope_name =
+ current_scope_->Value().CreateAnonymousScope("generate");
+ DeclareScopedElementAndDescend(node, scope_name,
SymbolMetaType::kGenerate);
}
}
+ void DeclareGenvarDeclaration(const SyntaxTreeNode &node) {
+ const auto *identifier_list =
+ GetSubtreeAsSymbol(node, NodeEnum::kGenvarDeclaration, 1);
+ if (!identifier_list) return;
+
+ const auto *list_node =
+ verible::down_cast<const SyntaxTreeNode *>(identifier_list);
+ for (const auto &child : list_node->children()) {
+ if (child && child->Kind() == verible::SymbolKind::kLeaf) {
+ const SyntaxTreeLeaf &leaf = verible::SymbolCastToLeaf(*child);
+ if (leaf.get().token_enum() == verilog_tokentype::SymbolIdentifier) {
+ EmplaceElementInCurrentScope(leaf, leaf.get().text(),
+ SymbolMetaType::kGenvar);
+ }
+ }
+ }
+ }
+
+ void DeclareSequentialBlock(const SyntaxTreeNode &node) {
+ // Procedural begin...end blocks introduce a new lexical scope.
+ // Extract optional label from the kBegin child for scope naming.
+ const verible::TokenInfo *label = nullptr;
+ if (!node.empty() && node.front() != nullptr) {
+ label = GetBeginLabelTokenInfo(*node.front());
+ }
+ const std::string_view scope_name =
+ label ? label->text()
+ : current_scope_->Value().CreateAnonymousScope("block");
+ DeclareScopedElementAndDescend(node, scope_name,
+ SymbolMetaType::kProceduralBlock);
+ }
+
+ void DeclareForLoop(const SyntaxTreeNode &node) {
+ // for-loop statements introduce a scope that encompasses both the
+ // initialization variables and the loop body.
+ const std::string_view scope_name =
+ current_scope_->Value().CreateAnonymousScope("for");
+ DeclareScopedElementAndDescend(node, scope_name,
+ SymbolMetaType::kLoopScope);
+ }
+
void DeclarePackage(const SyntaxTreeNode &package) {
const auto *token = GetPackageNameToken(package);
if (!token) return;
@@ -1432,6 +1516,43 @@
VLOG(2) << "end of " << __FUNCTION__;
}
+ // Declares a variable from a for-loop initialization (e.g. `int i = 0`).
+ // Handles both explicit `genvar` and standard typed declarations.
+ void DeclareForInitVariable(const SyntaxTreeNode &for_init) {
+ const verible::SyntaxTreeLeaf *genvar_keyword =
+ GetGenvarKeywordFromForInitialization(for_init);
+ if (genvar_keyword) {
+ const verible::SyntaxTreeLeaf *var_name =
+ GetVariableNameFromForInitialization(for_init);
+ if (var_name) {
+ EmplaceElementInCurrentScope(*var_name, var_name->get().text(),
+ SymbolMetaType::kGenvar);
+ }
+ Descend(for_init);
+ return;
+ }
+
+ const verible::SyntaxTreeNode *data_type =
+ GetDataTypeFromForInitialization(for_init);
+ if (data_type == nullptr) {
+ // not a variable declaration. Descend normally for references.
+ Descend(for_init);
+ return;
+ }
+ const verible::SyntaxTreeLeaf *var_name =
+ GetVariableNameFromForInitialization(for_init);
+ if (var_name == nullptr) {
+ Descend(for_init);
+ return;
+ }
+ DeclarationTypeInfo decl_type_info;
+ const ValueSaver<DeclarationTypeInfo *> save_type(&declaration_type_info_,
+ &decl_type_info);
+ Descend(for_init);
+ EmplaceTypedElementInCurrentScope(for_init, var_name->get().text(),
+ SymbolMetaType::kDataNetVariableInstance);
+ }
+
// Declare one (of potentially multiple) instances in a single declaration
// statement.
void DeclareInstance(const SyntaxTreeNode &instance) {
diff --git a/verible/verilog/analysis/symbol-table.h b/verible/verilog/analysis/symbol-table.h
index 6632b81..92fa30d 100644
--- a/verible/verilog/analysis/symbol-table.h
+++ b/verible/verilog/analysis/symbol-table.h
@@ -53,7 +53,9 @@
kRoot,
kClass,
kModule,
- kGenerate, // loop or conditional generate block
+ kGenerate, // loop or conditional generate block
+ kProceduralBlock, // begin...end block
+ kLoopScope, // for loop body and init
kPackage,
kParameter,
kTypeAlias, // typedef
@@ -64,6 +66,8 @@
kEnumType,
kEnumConstant,
kInterface,
+ kGenvar,
+ kGenerateLoop,
// The following enums represent classes/groups of the above types,
// and are used for validating metatypes of symbol references.
diff --git a/verible/verilog/analysis/symbol-table_test.cc b/verible/verilog/analysis/symbol-table_test.cc
index cec3c88..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).
@@ -713,6 +712,320 @@
}
}
+TEST_F(BuildSymbolTableTest, SeqBlockCreatesAnonymousScope) {
+ TestVerilogSourceFile src("foobar.sv",
+ "module m;\n"
+ " initial begin\n"
+ " int x;\n"
+ " end\n"
+ "endmodule\n");
+ const auto status = src.Parse();
+ ASSERT_TRUE(status.ok()) << status.message();
+ SymbolTable symbol_table(nullptr);
+ const SymbolTableNode &root_symbol(symbol_table.Root());
+
+ const auto build_diagnostics = BuildSymbolTable(src, &symbol_table);
+ EXPECT_EMPTY_STATUSES(build_diagnostics);
+
+ MUST_ASSIGN_LOOKUP_SYMBOL(module_node, root_symbol, "m");
+ EXPECT_EQ(module_node_info.metatype, SymbolMetaType::kModule);
+
+ // The begin...end block should create a child scope.
+ ASSERT_EQ(module_node.Children().size(), 1);
+ const SymbolTableNode &block(module_node.Children().begin()->second);
+ const SymbolInfo &block_info(block.Value());
+ EXPECT_EQ(block_info.metatype, SymbolMetaType::kProceduralBlock);
+
+ // 'x' should be declared inside the block scope, not module scope.
+ MUST_ASSIGN_LOOKUP_SYMBOL(var_x, block, "x");
+ EXPECT_EQ(var_x_info.metatype, SymbolMetaType::kDataNetVariableInstance);
+
+ {
+ std::vector<absl::Status> resolve_diagnostics;
+ symbol_table.Resolve(&resolve_diagnostics);
+ EXPECT_EMPTY_STATUSES(resolve_diagnostics);
+ }
+}
+
+TEST_F(BuildSymbolTableTest, SeqBlockLabeledScope) {
+ TestVerilogSourceFile src("foobar.sv",
+ "module m;\n"
+ " initial begin : my_block\n"
+ " int y;\n"
+ " end\n"
+ "endmodule\n");
+ const auto status = src.Parse();
+ ASSERT_TRUE(status.ok()) << status.message();
+ SymbolTable symbol_table(nullptr);
+ const SymbolTableNode &root_symbol(symbol_table.Root());
+
+ const auto build_diagnostics = BuildSymbolTable(src, &symbol_table);
+ EXPECT_EMPTY_STATUSES(build_diagnostics);
+
+ MUST_ASSIGN_LOOKUP_SYMBOL(module_node, root_symbol, "m");
+ EXPECT_EQ(module_node_info.metatype, SymbolMetaType::kModule);
+
+ // Labeled block should use the label as scope name.
+ MUST_ASSIGN_LOOKUP_SYMBOL(block_node, module_node, "my_block");
+ EXPECT_EQ(block_node_info.metatype, SymbolMetaType::kProceduralBlock);
+
+ MUST_ASSIGN_LOOKUP_SYMBOL(var_y, block_node, "y");
+ EXPECT_EQ(var_y_info.metatype, SymbolMetaType::kDataNetVariableInstance);
+
+ {
+ std::vector<absl::Status> resolve_diagnostics;
+ symbol_table.Resolve(&resolve_diagnostics);
+ EXPECT_EMPTY_STATUSES(resolve_diagnostics);
+ }
+}
+
+TEST_F(BuildSymbolTableTest, SeqBlockVariableShadowing) {
+ // Inner begin...end block declares 'x' which shadows the outer module-level
+ // 'x'. Both declarations should succeed (no "already defined" error).
+ TestVerilogSourceFile src("foobar.sv",
+ "module m;\n"
+ " int x;\n"
+ " initial begin\n"
+ " int x;\n"
+ " end\n"
+ "endmodule\n");
+ const auto status = src.Parse();
+ ASSERT_TRUE(status.ok()) << status.message();
+ SymbolTable symbol_table(nullptr);
+ const SymbolTableNode &root_symbol(symbol_table.Root());
+
+ const auto build_diagnostics = BuildSymbolTable(src, &symbol_table);
+ EXPECT_EMPTY_STATUSES(build_diagnostics);
+
+ MUST_ASSIGN_LOOKUP_SYMBOL(module_node, root_symbol, "m");
+ EXPECT_EQ(module_node_info.metatype, SymbolMetaType::kModule);
+
+ // Outer 'x' in module scope.
+ MUST_ASSIGN_LOOKUP_SYMBOL(outer_x, module_node, "x");
+ EXPECT_EQ(outer_x_info.metatype, SymbolMetaType::kDataNetVariableInstance);
+
+ // Block scope exists as a second child of module.
+ EXPECT_EQ(module_node.Children().size(), 2); // x + anonymous block
+ auto iter = module_node.Children().begin();
+ // Find the block child (skip past 'x').
+ while (iter != module_node.Children().end() &&
+ iter->second.Value().metatype != SymbolMetaType::kProceduralBlock) {
+ ++iter;
+ }
+ ASSERT_NE(iter, module_node.Children().end());
+ const SymbolTableNode &block(iter->second);
+
+ // Inner 'x' lives inside the block scope.
+ MUST_ASSIGN_LOOKUP_SYMBOL(inner_x, block, "x");
+ EXPECT_EQ(inner_x_info.metatype, SymbolMetaType::kDataNetVariableInstance);
+
+ {
+ std::vector<absl::Status> resolve_diagnostics;
+ symbol_table.Resolve(&resolve_diagnostics);
+ EXPECT_EMPTY_STATUSES(resolve_diagnostics);
+ }
+}
+
+TEST_F(BuildSymbolTableTest, GenerateForVariableScope) {
+ TestVerilogSourceFile src("foobar.sv",
+ "module m;\n"
+ " for (genvar i = 0; i < 5; i++) begin : gen_blk\n"
+ " int j;\n"
+ " end\n"
+ "endmodule\n");
+ const auto status = src.Parse();
+ ASSERT_TRUE(status.ok()) << status.message();
+ SymbolTable symbol_table(nullptr);
+ const SymbolTableNode &root_symbol(symbol_table.Root());
+
+ const auto build_diagnostics = BuildSymbolTable(src, &symbol_table);
+ EXPECT_EMPTY_STATUSES(build_diagnostics);
+
+ MUST_ASSIGN_LOOKUP_SYMBOL(module_node, root_symbol, "m");
+
+ // Module has one child: the generate for loop scope (anonymous).
+ ASSERT_EQ(module_node.Children().size(), 1);
+ const SymbolTableNode &for_scope(module_node.Children().begin()->second);
+ EXPECT_EQ(for_scope.Value().metatype, SymbolMetaType::kGenerateLoop);
+
+ // 'i' is declared in the for-loop scope.
+ MUST_ASSIGN_LOOKUP_SYMBOL(var_i, for_scope, "i");
+ EXPECT_EQ(var_i_info.metatype, SymbolMetaType::kGenvar);
+
+ // The generate for-loop body (begin...end) creates another nested scope
+ // containing 'j'.
+ MUST_ASSIGN_LOOKUP_SYMBOL(body_block, for_scope, "gen_blk");
+ EXPECT_EQ(body_block_info.metatype, SymbolMetaType::kGenerate);
+
+ MUST_ASSIGN_LOOKUP_SYMBOL(var_j, body_block, "j");
+ EXPECT_EQ(var_j_info.metatype, SymbolMetaType::kDataNetVariableInstance);
+
+ {
+ std::vector<absl::Status> resolve_diagnostics;
+ symbol_table.Resolve(&resolve_diagnostics);
+ EXPECT_EMPTY_STATUSES(resolve_diagnostics);
+ }
+}
+
+TEST_F(BuildSymbolTableTest, GenerateForVariableScopeAnonymous) {
+ TestVerilogSourceFile src("foobar.sv",
+ "module m;\n"
+ " for (genvar i = 0; i < 5; i++) begin\n"
+ " int j;\n"
+ " end\n"
+ "endmodule\n");
+ const auto status = src.Parse();
+ ASSERT_TRUE(status.ok()) << status.message();
+ SymbolTable symbol_table(nullptr);
+ const SymbolTableNode &root_symbol(symbol_table.Root());
+
+ const auto build_diagnostics = BuildSymbolTable(src, &symbol_table);
+ EXPECT_EMPTY_STATUSES(build_diagnostics);
+
+ MUST_ASSIGN_LOOKUP_SYMBOL(module_node, root_symbol, "m");
+ ASSERT_EQ(module_node.Children().size(), 1);
+ const SymbolTableNode &for_scope(module_node.Children().begin()->second);
+ EXPECT_EQ(for_scope.Value().metatype, SymbolMetaType::kGenerateLoop);
+
+ MUST_ASSIGN_LOOKUP_SYMBOL(var_i, for_scope, "i");
+ EXPECT_EQ(var_i_info.metatype, SymbolMetaType::kGenvar);
+
+ const SymbolTableNode *body_block = nullptr;
+ for (const auto &child : for_scope.Children()) {
+ if (child.second.Value().metatype == SymbolMetaType::kGenerate) {
+ body_block = &child.second;
+ break;
+ }
+ }
+ ASSERT_NE(body_block, nullptr);
+ MUST_ASSIGN_LOOKUP_SYMBOL(var_j, *body_block, "j");
+ EXPECT_EQ(var_j_info.metatype, SymbolMetaType::kDataNetVariableInstance);
+}
+
+TEST_F(BuildSymbolTableTest, StandaloneGenvarDeclaration) {
+ TestVerilogSourceFile src("foobar.sv",
+ "module m;\n"
+ " genvar i, j;\n"
+ "endmodule\n");
+ const auto status = src.Parse();
+ ASSERT_TRUE(status.ok()) << status.message();
+ SymbolTable symbol_table(nullptr);
+ const SymbolTableNode &root_symbol(symbol_table.Root());
+
+ const auto build_diagnostics = BuildSymbolTable(src, &symbol_table);
+ EXPECT_EMPTY_STATUSES(build_diagnostics);
+
+ MUST_ASSIGN_LOOKUP_SYMBOL(module_node, root_symbol, "m");
+ MUST_ASSIGN_LOOKUP_SYMBOL(var_i, module_node, "i");
+ EXPECT_EQ(var_i_info.metatype, SymbolMetaType::kGenvar);
+ MUST_ASSIGN_LOOKUP_SYMBOL(var_j, module_node, "j");
+ EXPECT_EQ(var_j_info.metatype, SymbolMetaType::kGenvar);
+}
+
+TEST_F(BuildSymbolTableTest, ForLoopVariableScope) {
+ TestVerilogSourceFile src("foobar.sv",
+ "module m;\n"
+ " initial begin\n"
+ " for (int i = 0; i < 5; i++) begin\n"
+ " int j;\n"
+ " end\n"
+ " end\n"
+ "endmodule\n");
+ const auto status = src.Parse();
+ ASSERT_TRUE(status.ok()) << status.message();
+ SymbolTable symbol_table(nullptr);
+ const SymbolTableNode &root_symbol(symbol_table.Root());
+
+ const auto build_diagnostics = BuildSymbolTable(src, &symbol_table);
+ EXPECT_EMPTY_STATUSES(build_diagnostics);
+
+ MUST_ASSIGN_LOOKUP_SYMBOL(module_node, root_symbol, "m");
+
+ // Module has one child: the initial block scope.
+ ASSERT_EQ(module_node.Children().size(), 1);
+ const SymbolTableNode &init_block(module_node.Children().begin()->second);
+ EXPECT_EQ(init_block.Value().metatype, SymbolMetaType::kProceduralBlock);
+
+ // The initial block has one child: the for-loop scope.
+ ASSERT_EQ(init_block.Children().size(), 1);
+ const SymbolTableNode &for_scope(init_block.Children().begin()->second);
+ EXPECT_EQ(for_scope.Value().metatype, SymbolMetaType::kLoopScope);
+
+ // 'i' is declared in the for-loop scope.
+ MUST_ASSIGN_LOOKUP_SYMBOL(var_i, for_scope, "i");
+ EXPECT_EQ(var_i_info.metatype, SymbolMetaType::kDataNetVariableInstance);
+
+ // The for-loop body (begin...end) creates another nested scope containing
+ // 'j'. Find the block scope child (skip past 'i').
+ const SymbolTableNode *body_block = nullptr;
+ for (const auto &child : for_scope.Children()) {
+ if (child.second.Value().metatype == SymbolMetaType::kProceduralBlock) {
+ body_block = &child.second;
+ break;
+ }
+ }
+ ASSERT_NE(body_block, nullptr);
+ MUST_ASSIGN_LOOKUP_SYMBOL(var_j, *body_block, "j");
+ EXPECT_EQ(var_j_info.metatype, SymbolMetaType::kDataNetVariableInstance);
+
+ {
+ std::vector<absl::Status> resolve_diagnostics;
+ symbol_table.Resolve(&resolve_diagnostics);
+ EXPECT_EMPTY_STATUSES(resolve_diagnostics);
+ }
+}
+
+TEST_F(BuildSymbolTableTest, ForLoopVariableShadowing) {
+ // Module-level 'i' and for-loop 'i' should not collide.
+ TestVerilogSourceFile src("foobar.sv",
+ "module m;\n"
+ " int i;\n"
+ " initial begin\n"
+ " for (int i = 0; i < 5; i++) begin\n"
+ " end\n"
+ " end\n"
+ "endmodule\n");
+ const auto status = src.Parse();
+ ASSERT_TRUE(status.ok()) << status.message();
+ SymbolTable symbol_table(nullptr);
+ const SymbolTableNode &root_symbol(symbol_table.Root());
+
+ const auto build_diagnostics = BuildSymbolTable(src, &symbol_table);
+ EXPECT_EMPTY_STATUSES(build_diagnostics);
+
+ MUST_ASSIGN_LOOKUP_SYMBOL(module_node, root_symbol, "m");
+
+ // Module-level 'i' exists.
+ MUST_ASSIGN_LOOKUP_SYMBOL(outer_i, module_node, "i");
+ EXPECT_EQ(outer_i_info.metatype, SymbolMetaType::kDataNetVariableInstance);
+
+ // Find the for-loop scope (nested inside the initial block scope).
+ const SymbolTableNode *for_scope = nullptr;
+ for (const auto &child : module_node.Children()) {
+ if (child.second.Value().metatype == SymbolMetaType::kProceduralBlock) {
+ // This is the initial block. Search inside for the for-loop scope.
+ for (const auto &grandchild : child.second.Children()) {
+ if (grandchild.second.Value().metatype == SymbolMetaType::kLoopScope) {
+ for_scope = &grandchild.second;
+ break;
+ }
+ }
+ break;
+ }
+ }
+ ASSERT_NE(for_scope, nullptr);
+
+ // For-loop 'i' lives in the for-loop scope.
+ MUST_ASSIGN_LOOKUP_SYMBOL(loop_i, *for_scope, "i");
+ EXPECT_EQ(loop_i_info.metatype, SymbolMetaType::kDataNetVariableInstance);
+
+ {
+ std::vector<absl::Status> resolve_diagnostics;
+ symbol_table.Resolve(&resolve_diagnostics);
+ EXPECT_EMPTY_STATUSES(resolve_diagnostics);
+ }
+}
+
TEST_F(BuildSymbolTableTest, ModuleDeclarationWithPorts) {
TestVerilogSourceFile src("foobar.sv",
"module m (\n"
diff --git a/verible/verilog/analysis/verilog-filelist.h b/verible/verilog/analysis/verilog-filelist.h
index 0bb474a..915eb52 100644
--- a/verible/verilog/analysis/verilog-filelist.h
+++ b/verible/verilog/analysis/verilog-filelist.h
@@ -27,7 +27,7 @@
// TODO(karimtera): Using "MacroDefiniton" struct might be better.
struct TextMacroDefinition {
TextMacroDefinition(std::string name, std::string value)
- : name(std::move(name)), value(std::move(value)){};
+ : name(std::move(name)), value(std::move(value)) {};
std::string name;
std::string value;
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/align.cc b/verible/verilog/formatting/align.cc
index 1f7f472..6640adb 100644
--- a/verible/verilog/formatting/align.cc
+++ b/verible/verilog/formatting/align.cc
@@ -632,7 +632,9 @@
kDontCare = 0,
kNamedActualParameters,
kNamedActualPorts,
- kParameterDeclaration,
+ kParameterDeclaration, // formal parameter list (#(...))
+ kBodyParameterDeclaration, // parameter/localparam in module/generate/
+ // package/interface body
kPortDeclaration,
kStructUnionMember,
kDataDeclaration, // net/variable declarations
@@ -674,6 +676,13 @@
return AlignClassify(AlignmentGroupAction::kIgnore);
}
const SyntaxTreeNode &node = verible::SymbolCastToNode(*origin);
+ // Align body-level parameter/localparam declarations.
+ if (node.MatchesTag(NodeEnum::kParamDeclaration)) {
+ return AlignClassify(
+ AlignmentGroupAction::kMatch,
+ AlignableSyntaxSubtype::kBodyParameterDeclaration);
+ }
+
// Align net/variable declarations.
if (IsAlignableDeclaration(node)) {
return AlignClassify(AlignmentGroupAction::kMatch,
@@ -837,7 +846,7 @@
}
case NodeEnum::kDimensionSlice:
case NodeEnum::kDimensionAssociativeType: {
- // all of these cases cover packed and unpacked dimensions
+ // All of these cases cover packed and unpacked dimensions
ReserveNewColumn(node, FlushLeft);
break;
}
@@ -951,7 +960,6 @@
CHECK_EQ(node.size(), 5);
auto *column = ABSL_DIE_IF_NULL(ReserveNewColumn(node, FlushLeft));
- SyntaxTreePath np;
ReserveNewColumn(column, *node[0], FlushLeft); // '['
auto *value_subcolumn =
@@ -985,9 +993,8 @@
}
};
-// This class marks up token-subranges in formal parameter declarations for
-// alignment.
-// e.g. "localparam int Width = 5;"
+// This class marks up token-subranges in formal and body-level parameter
+// declarations for alignment. e.g. "localparam int Width = 5;"
class ParameterDeclarationColumnSchemaScanner
: public VerilogColumnSchemaScanner {
public:
@@ -1061,7 +1068,7 @@
break;
}
- // Sometimes the parameter indentifier which is of token SymbolIdentifier
+ // Sometimes the parameter identifier which is of token SymbolIdentifier
// can appear at different paths depending on the parameter type. Make
// them aligned so they fall under the same column.
case verilog_tokentype::SymbolIdentifier: {
@@ -1090,7 +1097,7 @@
break;
}
- // Align packed and unpacked dimenssions
+ // Align packed and unpacked dimensions
case '[': {
if (verilog::analysis::ContextIsInsideDeclarationDimensions(
Context()) &&
@@ -1456,6 +1463,11 @@
ParameterDeclarationColumnSchemaScanner>(non_tree_column_scanner),
function_from_pointer_to_member(
&FormatStyle::formal_parameters_alignment)}},
+ {AlignableSyntaxSubtype::kBodyParameterDeclaration,
+ {UnstyledAlignmentCellScannerGenerator<
+ ParameterDeclarationColumnSchemaScanner>(non_tree_column_scanner),
+ function_from_pointer_to_member(
+ &FormatStyle::parameter_declaration_alignment)}},
{AlignableSyntaxSubtype::kPortDeclaration,
{UnstyledAlignmentCellScannerGenerator<
PortDeclarationColumnSchemaScanner>(non_tree_column_scanner),
@@ -1598,8 +1610,9 @@
static std::vector<AlignablePartitionGroup> AlignModuleItems(
const TokenPartitionRange &full_range, const FormatStyle &vstyle) {
- // Currently, this only handles data/net/variable declarations.
- // TODO(b/161814377): align continuous assignments
+ // Applies to module/interface, generate, and package item lists.
+ // Handles data/net/variable declarations, parameter/localparam
+ // declarations, and continuous assignment statements.
auto group_extractor = [&vstyle](const TokenPartitionRange &range) {
return GetConsecutiveModuleItemGroups(range,
vstyle.alignment_group_boundary);
@@ -1635,6 +1648,10 @@
&IgnoreCommentsAndPreprocessingDirectives, full_range, vstyle);
}
+// Aligns formal parameters in #(...) headers (module/interface/class port
+// parameter lists). Body-level parameter/localparam declarations are
+// handled by AlignModuleItems which dispatches through
+// GetConsecutiveModuleItemGroups.
static std::vector<AlignablePartitionGroup> AlignParameterDeclarations(
const TokenPartitionRange &full_range, const FormatStyle &vstyle) {
return ExtractAlignablePartitionGroups(
@@ -1681,8 +1698,11 @@
{NodeEnum::kStructUnionMemberList, &AlignStructUnionMembers},
{NodeEnum::kActualParameterByNameList, &AlignActualNamedParameters},
{NodeEnum::kPortActualList, &AlignActualNamedPorts},
+ // module/interface bodies
{NodeEnum::kModuleItemList, &AlignModuleItems},
{NodeEnum::kGenerateItemList, &AlignModuleItems},
+ // package bodies
+ {NodeEnum::kPackageItemList, &AlignModuleItems},
{NodeEnum::kFormalParameterList, &AlignParameterDeclarations},
{NodeEnum::kClassItems, &AlignClassItems},
// various case-like constructs:
diff --git a/verible/verilog/formatting/format-style-init.cc b/verible/verilog/formatting/format-style-init.cc
index 0fa91af..a6d010b 100644
--- a/verible/verilog/formatting/format-style-init.cc
+++ b/verible/verilog/formatting/format-style-init.cc
@@ -105,13 +105,20 @@
ABSL_FLAG(AlignmentPolicy, named_port_alignment,
AlignmentPolicy::kInferUserIntent,
"Format named port connections: {align,flush-left,preserve,infer}");
-ABSL_FLAG(
- AlignmentPolicy, module_net_variable_alignment, //
- AlignmentPolicy::kInferUserIntent,
- "Format net/variable declarations: {align,flush-left,preserve,infer}");
+ABSL_FLAG(AlignmentPolicy, module_net_variable_alignment,
+ AlignmentPolicy::kInferUserIntent,
+ "Format net/variable declarations in module, generate, "
+ "interface, and package bodies: {align,flush-left,preserve,infer}");
ABSL_FLAG(AlignmentPolicy, formal_parameters_alignment,
AlignmentPolicy::kInferUserIntent,
- "Format formal parameters: {align,flush-left,preserve,infer}");
+ "Format formal parameters in module/interface/class headers "
+ "(inside #(...)): {align,flush-left,preserve,infer}");
+ABSL_FLAG(AlignmentPolicy, parameter_declaration_alignment,
+ AlignmentPolicy::kInferUserIntent,
+ "Format parameter/localparam declarations in module, generate, "
+ "interface, and package bodies "
+ "(class body parameter declarations are NOT affected): "
+ "{align,flush-left,preserve,infer}");
ABSL_FLAG(AlignmentPolicy, class_member_variable_alignment,
AlignmentPolicy::kInferUserIntent,
"Format class member variables: {align,flush-left,preserve,infer}");
@@ -123,7 +130,8 @@
"Align distribution items: {align,flush-left,preserve,infer}");
ABSL_FLAG(AlignmentPolicy, assignment_statement_alignment,
AlignmentPolicy::kInferUserIntent,
- "Format various assignments: {align,flush-left,preserve,infer}");
+ "Format various assignments in module, generate, interface, "
+ "and package bodies: {align,flush-left,preserve,infer}");
ABSL_FLAG(AlignmentPolicy, enum_assignment_statement_alignment,
AlignmentPolicy::kInferUserIntent,
"Format assignments with enums: {align,flush-left,preserve,infer}");
@@ -132,6 +140,13 @@
"Use compact binary expressions inside indexing / bit selection "
"operators");
+ABSL_FLAG(bool, class_parameter_space, false,
+ "If true, keep/insert a space before '#' in a class"
+ "parameterized typedef, e.g. \"typedef my_class #(.P(P)) "
+ "my_class_t;\". If false (default), no space is inserted, "
+ "matching the convention used for IEEE parameterized class "
+ "instantiations, e.g. \"type#(params...)::method(...)\".");
+
ABSL_FLAG(bool, wrap_end_else_clauses, false,
"Split end and else keywords into separate lines");
@@ -167,7 +182,7 @@
#define STYLE_FROM_FLAG(name) style->name = absl::GetFlag(FLAGS_##name)
- // Simply in the sequence as declared in struct FormatStyle
+ // In the same sequence as declared in struct FormatStyle
STYLE_FROM_FLAG(port_declarations_indentation);
STYLE_FROM_FLAG(port_declarations_alignment);
STYLE_FROM_FLAG(struct_union_members_alignment);
@@ -180,6 +195,7 @@
STYLE_FROM_FLAG(enum_assignment_statement_alignment);
STYLE_FROM_FLAG(formal_parameters_indentation);
STYLE_FROM_FLAG(formal_parameters_alignment);
+ STYLE_FROM_FLAG(parameter_declaration_alignment);
STYLE_FROM_FLAG(class_member_variable_alignment);
STYLE_FROM_FLAG(case_items_alignment);
STYLE_FROM_FLAG(distribution_items_alignment);
@@ -188,6 +204,7 @@
STYLE_FROM_FLAG(try_wrap_long_lines);
STYLE_FROM_FLAG(expand_coverpoints);
STYLE_FROM_FLAG(compact_indexing_and_selections);
+ STYLE_FROM_FLAG(class_parameter_space);
STYLE_FROM_FLAG(wrap_end_else_clauses);
STYLE_FROM_FLAG(alignment_group_boundary);
diff --git a/verible/verilog/formatting/format-style.h b/verible/verilog/formatting/format-style.h
index e73cf14..55347c5 100644
--- a/verible/verilog/formatting/format-style.h
+++ b/verible/verilog/formatting/format-style.h
@@ -25,7 +25,7 @@
namespace verilog {
namespace formatter {
-// Controls what breaks alignment groups for module items, statements,
+// Control what breaks alignment groups for module items, statements,
// and class items.
enum class AlignmentGroupBoundary {
// No additional group splitting (default, current behavior).
@@ -52,11 +52,9 @@
FormatStyle(const FormatStyle &) = default;
- /*
- * InitializeFromFlags() [format_style_init.h] provides flags that are
- * named like these fields and allow configuration on the command line.
- * So field foo here can be configured with flag --foo
- */
+ // InitializeFromFlags() [format-style-init.cc] provides flags that are
+ // named like these fields and allow configuration on the command line.
+ // So field foo here can be configured with flag --foo.
// TODO(hzeller): some of these are plural, some singular. Come up with
// a consistent scheme.
@@ -86,25 +84,37 @@
AlignmentPolicy named_port_alignment = AlignmentPolicy::kAlign;
// Control how module-local net/variable declarations are formatted.
+ // Applies in module, generate, interface, and package bodies.
// Internal tests assume these are forced to kAlign.
AlignmentPolicy module_net_variable_alignment = AlignmentPolicy::kAlign;
// Control how various assignment statements should be aligned.
+ // Applies in module, generate, interface, and package bodies.
// This covers: continuous assignment statements,
// blocking, and nonblocking assignments.
// Internal tests assume these are forced to kAlign.
AlignmentPolicy assignment_statement_alignment = AlignmentPolicy::kAlign;
- // Assignment within enumerations.
+ // Control how assignments in enumerations should be aligned.
AlignmentPolicy enum_assignment_statement_alignment = AlignmentPolicy::kAlign;
// Control indentation amount for formal parameter declarations.
IndentationStyle formal_parameters_indentation = IndentationStyle::kWrap;
- // Control how formal parameters in modules/interfaces/classes are formatted.
+ // Control how formal parameters in module/interface/class headers
+ // (inside #(...)) are formatted. For parameter/localparam
+ // declarations in module, generate, interface, and package bodies,
+ // see parameter_declaration_alignment.
// Internal tests assume these are forced to kAlign.
AlignmentPolicy formal_parameters_alignment = AlignmentPolicy::kAlign;
+ // Control how parameter/localparam declarations are formatted.
+ // Applies in module, generate, interface, and package bodies.
+ // Class body parameter declarations are not affected.
+ // For formal parameters in #(...) headers, see formal_parameters_alignment.
+ // Internal tests assume these are forced to kAlign.
+ AlignmentPolicy parameter_declaration_alignment = AlignmentPolicy::kAlign;
+
// Control how class member variables are formatted.
// Internal tests assume these are forced to kAlign.
AlignmentPolicy class_member_variable_alignment = AlignmentPolicy::kAlign;
@@ -120,7 +130,7 @@
bool port_declarations_right_align_packed_dimensions = false;
bool port_declarations_right_align_unpacked_dimensions = false;
- // Controls what breaks alignment groups for module items, statements,
+ // Control what breaks alignment groups for module items, statements,
// and class items.
AlignmentGroupBoundary alignment_group_boundary =
AlignmentGroupBoundary::kNone;
@@ -136,10 +146,13 @@
// Compact binary expressions inside indexing / bit selection operators
bool compact_indexing_and_selections = true;
+ // Keep/insert a space before '#' in a parameterized class typedef
+ bool class_parameter_space = false;
+
// Split with a \n end and else clauses
bool wrap_end_else_clauses = false;
- // -- Note: when adding new fields, add them in format_style_init.cc
+ // -- Note: When adding new fields, add them in format-style-init.cc
// TODO(fangism): introduce the following knobs:
//
@@ -174,12 +187,16 @@
: indentation_spaces;
}
+ // -- Note: When adding a new AlignmentPolicy field, add it here
+ // and to InitializeFromFlags in format-style-init.cc.
void ApplyToAllAlignmentPolicies(AlignmentPolicy policy) {
port_declarations_alignment = policy;
+ struct_union_members_alignment = policy;
named_parameter_alignment = policy;
named_port_alignment = policy;
module_net_variable_alignment = policy;
formal_parameters_alignment = policy;
+ parameter_declaration_alignment = policy;
class_member_variable_alignment = policy;
case_items_alignment = policy;
assignment_statement_alignment = policy;
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 e885d74..bd0f775 100644
--- a/verible/verilog/formatting/formatter_test.cc
+++ b/verible/verilog/formatting/formatter_test.cc
@@ -162,6 +162,12 @@
"`define BAR\n",
"`define FOO\n"
"`define BAR\n"},
+ {"`define FOO_``BAR 1\n", "`define FOO_``BAR 1\n"},
+ {"`define FOO_```BAR 1\n", "`define FOO_```BAR 1\n"},
+ {"`define FOO ``BAR\n", "`define FOO ``BAR\n"},
+ {"`define A``B``C 2\n", "`define A``B``C 2\n"},
+ {"`define A(x)``y\n", "`define A(x) ``y\n"},
+ {"`define FOO_``BAR\n", "`define FOO_``BAR\n"},
{"`ifndef FOO\n"
"`endif // FOO\n",
"`ifndef FOO\n"
@@ -1503,6 +1509,119 @@
" int b\n" // direction missing
");\n"
"endmodule\n"},
+ {"module t;\n"
+ " input bit i_bit;\n"
+ "input byte i_byte ;\n"
+ "input chandle i_chandle;\n"
+ "input event i_event;\n"
+ "input int i_int;\n"
+ "input integer i_inte;\n"
+ "input longint i_longint;\n"
+ "input real i_real;\n"
+ "input realtime i_realtime;\n"
+ "input shortint i_shortint;\n"
+ "input shortreal i_shortreal;\n"
+ "input string i_string;\n"
+ "input time i_time;\n"
+ " output bit o_bit;\n"
+ "output byte o_byte ; \n"
+ "output chandle o_chandle;\n"
+ "output event o_event;\n"
+ "output int o_int;\n"
+ "output integer o_inte;\n"
+ "output longint o_longint;\n"
+ "output real o_real;\n"
+ "output realtime o_realtime;\n"
+ "output shortint o_shortint;\n"
+ "output shortreal o_shortreal;\n"
+ "output string o_string;\n"
+ "output time o_time;\n"
+ "endmodule\n",
+ "module t;\n"
+ " input bit i_bit;\n"
+ " input byte i_byte;\n"
+ " input chandle i_chandle;\n"
+ " input event i_event;\n"
+ " input int i_int;\n"
+ " input integer i_inte;\n"
+ " input longint i_longint;\n"
+ " input real i_real;\n"
+ " input realtime i_realtime;\n"
+ " input shortint i_shortint;\n"
+ " input shortreal i_shortreal;\n"
+ " input string i_string;\n"
+ " input time i_time;\n"
+ " output bit o_bit;\n"
+ " output byte o_byte;\n"
+ " output chandle o_chandle;\n"
+ " output event o_event;\n"
+ " output int o_int;\n"
+ " output integer o_inte;\n"
+ " output longint o_longint;\n"
+ " output real o_real;\n"
+ " output realtime o_realtime;\n"
+ " output shortint o_shortint;\n"
+ " output shortreal o_shortreal;\n"
+ " output string o_string;\n"
+ " output time o_time;\n"
+ "endmodule\n"},
+ {"module t (\n"
+ " input bit i_bit,\n"
+ "input byte i_byte ,\n"
+ "input chandle i_chandle,\n"
+ "input event i_event,\n"
+ "input int i_int,\n"
+ "input integer i_inte,\n"
+ "input longint i_longint,\n"
+ "input real i_real,\n"
+ "input realtime i_realtime,\n"
+ "input shortint i_shortint,\n"
+ "input shortreal i_shortreal,\n"
+ "input string i_string,\n"
+ "input time i_time,\n"
+ " output bit o_bit,\n"
+ "output byte o_byte , \n"
+ "output chandle o_chandle,\n"
+ "output event o_event,\n"
+ "output int o_int,\n"
+ "output integer o_inte,\n"
+ "output longint o_longint,\n"
+ "output real o_real,\n"
+ "output realtime o_realtime,\n"
+ "output shortint o_shortint,\n"
+ "output shortreal o_shortreal,\n"
+ "output string o_string,\n"
+ "output time o_time);\n"
+ "endmodule\n",
+ "module t (\n"
+ " input bit i_bit,\n"
+ " input byte i_byte,\n"
+ " input chandle i_chandle,\n"
+ " input event i_event,\n"
+ " input int i_int,\n"
+ " input integer i_inte,\n"
+ " input longint i_longint,\n"
+ " input real i_real,\n"
+ " input realtime i_realtime,\n"
+ " input shortint i_shortint,\n"
+ " input shortreal i_shortreal,\n"
+ " input string i_string,\n"
+ " input time i_time,\n"
+ " output bit o_bit,\n"
+ " output byte o_byte,\n"
+ " output chandle o_chandle,\n"
+ " output event o_event,\n"
+ " output int o_int,\n"
+ " output integer o_inte,\n"
+ " output longint o_longint,\n"
+ " output real o_real,\n"
+ " output realtime o_realtime,\n"
+ " output shortint o_shortint,\n"
+ " output shortreal o_shortreal,\n"
+ " output string o_string,\n"
+ " output time o_time\n"
+ ");\n"
+ "endmodule\n"},
{"module m;foo bar(.baz({larry, moe, curly}));endmodule",
"module m;\n"
" foo bar (.baz({larry, moe, curly}));\n"
@@ -3885,6 +4004,22 @@
" endtask\n"
" // class is about to end\n"
"endclass\n"},
+
+ // interface class test cases
+ {"interface class Foo;\nendclass\n",
+ "interface class Foo;\n"
+ "endclass\n"},
+ {"interface class Foo ; endclass\n",
+ "interface class Foo;\n"
+ "endclass\n"},
+ {"interface class Foo extends Bar , Baz ;\nendclass\n",
+ "interface class Foo extends Bar, Baz;\n"
+ "endclass\n"},
+ {"interface class Foo;\n pure virtual task foo ( ) ; \nendclass\n",
+ "interface class Foo;\n"
+ " pure virtual task foo();\n"
+ "endclass\n"},
+
// class property alignment test cases
{"class c;\n"
"int foo ;\n"
@@ -4343,6 +4478,42 @@
" .L(L),\n"
" .W(W)\n"
") bar_t;\n"},
+ // By default (class_parameter_space == false), no space before '#'
+ {"typedef dv_base_env_cov #(.CFG_T(tl_agent_env_cfg)) tl_agent_env_cov;\n",
+ "typedef dv_base_env_cov#(\n"
+ " .CFG_T(tl_agent_env_cfg)\n"
+ ") tl_agent_env_cov;\n"},
+ {"typedef dv_base_env_cov#(.CFG_T(tl_agent_env_cfg)) tl_agent_env_cov;\n",
+ "typedef dv_base_env_cov#(\n"
+ " .CFG_T(tl_agent_env_cfg)\n"
+ ") tl_agent_env_cov;\n"},
+ // single short parameter stays on one line
+ {"typedef my_class #(.P(P)) my_class_t;\n",
+ "typedef my_class#(.P(P)) my_class_t;\n"},
+
+ // let declarations each stay on their own line
+ {"module t;\n"
+ "let OFF = 4;\n"
+ "let UNIQUE = 32;\n"
+ "let PP(a) = 30 + a;\n"
+ "endmodule\n",
+ "module t;\n"
+ " let OFF = 4;\n"
+ " let UNIQUE = 32;\n"
+ " let PP(a) = 30 + a;\n"
+ "endmodule\n"},
+
+ // let declarations each stay on their own line
+ {"module t;\n"
+ "let OFF = 4;\n"
+ "let UNIQUE = 32;\n"
+ "let PP(a) = 30 + a;\n"
+ "endmodule\n",
+ "module t;\n"
+ " let OFF = 4;\n"
+ " let UNIQUE = 32;\n"
+ " let PP(a) = 30 + a;\n"
+ "endmodule\n"},
// package test cases
{"package fedex;localparam int www=3 ;endpackage : fedex\n",
@@ -4559,6 +4730,15 @@
" for (int i = 0; i < f(m); i--) begin\n"
" end\n"
"endfunction\n"},
+ {// for loop with an attribute instance in the initializer.
+ // Regression: this used to abort with a CHECK failure while reshaping
+ // the kForSpec partitions when an attribute appears in the header.
+ "module m; initial for(int i=0(* a *);i<4;i++) x=i; endmodule",
+ "module m;\n"
+ " initial\n"
+ " for (int i = 0 (* a *); i < 4; i++)\n"
+ " x = i;\n"
+ "endmodule\n"},
{// forever loop
"function\nvoid\tforevah;forever begin "
"++k\n;end endfunction\n",
@@ -8125,6 +8305,22 @@
" bit [31:0] second;\n"
" generic_type_name_t third;\n"
"} type_t;\n"},
+ {"typedef union soft packed {\n"
+ "bit [3:0] first; bit [31:0] second; generic_type_name_t third;\n"
+ "} type_t;",
+ "typedef union soft packed {\n"
+ " bit [3:0] first;\n"
+ " bit [31:0] second;\n"
+ " generic_type_name_t third;\n"
+ "} type_t;\n"},
+ {"typedef union soft {\n"
+ "bit [3:0] first; bit [31:0] second; generic_type_name_t third;\n"
+ "} type_t;",
+ "typedef union soft {\n"
+ " bit [3:0] first;\n"
+ " bit [31:0] second;\n"
+ " generic_type_name_t third;\n"
+ "} type_t;\n"},
{"typedef struct {\n"
"// comment\n"
"bit [3:0] first; bit [31:0] second; generic_type_name_t third;\n"
@@ -18624,6 +18820,50 @@
}
}
+static constexpr FormatterTestCase
+ kSpaceBeforeHashInUnqualifiedTypedefTestCases[] = {
+ // unqualified parameterized type keeps a space before '#'
+ {"typedef dv_base_env_cov #(.CFG_T(tl_agent_env_cfg)) "
+ "tl_agent_env_cov;\n",
+ "typedef dv_base_env_cov #(\n"
+ " .CFG_T(tl_agent_env_cfg)\n"
+ ") tl_agent_env_cov;\n"},
+ // ... and is inserted when absent
+ {"typedef dv_base_env_cov#(.CFG_T(tl_agent_env_cfg)) "
+ "tl_agent_env_cov;\n",
+ "typedef dv_base_env_cov #(\n"
+ " .CFG_T(tl_agent_env_cfg)\n"
+ ") tl_agent_env_cov;\n"},
+ // single short parameter stays on one line
+ {"typedef my_class #(.P(P)) my_class_t;\n",
+ "typedef my_class #(.P(P)) my_class_t;\n"},
+ // package-qualified types are unaffected (no space before '#')
+ {"typedef foo_pkg::baz_t#(.L(L), .W(W)) bar_t;\n",
+ "typedef foo_pkg::baz_t#(\n"
+ " .L(L),\n"
+ " .W(W)\n"
+ ") bar_t;\n"},
+};
+
+TEST(FormatterEndToEndTest, SpaceBeforeHashInUnqualifiedTypedefTestCases) {
+ // Use a fixed style.
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.class_parameter_space = true;
+
+ for (const auto &test_case : kSpaceBeforeHashInUnqualifiedTypedefTestCases) {
+ VLOG(1) << "code-to-format:\n" << test_case.input << "<EOF>";
+ std::ostringstream stream;
+ const auto status =
+ FormatVerilog(test_case.input, "<filename>", style, stream);
+ // Require these test cases to be valid.
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+ }
+}
+
static constexpr FormatterTestCase kFunctionCallsWithComments[] = {
{// no comments
"module foo;\n"
@@ -19216,6 +19456,1737 @@
}
}
+// 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).
+TEST(FormatterEndToEndTest, EndElseIfWithEOLCommentConverges) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// Comment on its own line between end and else if
+ "module m;\n"
+ " always_comb begin\n"
+ " case (state)\n"
+ " STATE_A: begin\n"
+ " if (cond_aaaa) next_state_value = STATE_B;\n"
+ " else if (cond_bbbb) begin\n"
+ " next_state_value = STATE_B;\n"
+ " end\n"
+ " // xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\n"
+ " else if (cond_cccc) next_state_value = STATE_C;\n"
+ " end\n"
+ " endcase\n"
+ " end\n"
+ "endmodule\n",
+ "module m;\n"
+ " always_comb begin\n"
+ " case (state)\n"
+ " STATE_A: begin\n"
+ " if (cond_aaaa) next_state_value = STATE_B;\n"
+ " else if (cond_bbbb) begin\n"
+ " next_state_value = STATE_B;\n"
+ " end // xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\n"
+ " else if (cond_cccc) next_state_value = STATE_C;\n"
+ " end\n"
+ " endcase\n"
+ " end\n"
+ "endmodule\n"},
+ {// Same construct with comment already on the end line
+ "module m;\n"
+ " always_comb begin\n"
+ " case (state)\n"
+ " STATE_A: begin\n"
+ " if (cond_aaaa) next_state_value = STATE_B;\n"
+ " else if (cond_bbbb) begin\n"
+ " next_state_value = STATE_B;\n"
+ " end // xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\n"
+ " else if (cond_cccc) next_state_value = STATE_C;\n"
+ " end\n"
+ " endcase\n"
+ " end\n"
+ "endmodule\n",
+ "module m;\n"
+ " always_comb begin\n"
+ " case (state)\n"
+ " STATE_A: begin\n"
+ " if (cond_aaaa) next_state_value = STATE_B;\n"
+ " else if (cond_bbbb) begin\n"
+ " next_state_value = STATE_B;\n"
+ " end // xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\n"
+ " else if (cond_cccc) next_state_value = STATE_C;\n"
+ " end\n"
+ " endcase\n"
+ " end\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;
+ }
+}
+
+// Verify kAlign behavior for body-level param/localparam declarations
+// in module and package bodies.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentBasics) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// localparam alignment in module body
+ "module m;\n"
+ "localparam foo = 4'b0000;\n"
+ "localparam barr = 4'b0010;\n"
+ "localparam baaaaz = 4'b0111;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 4'b0000;\n"
+ " localparam barr = 4'b0010;\n"
+ " localparam baaaaz = 4'b0111;\n"
+ "endmodule\n"},
+ {// localparam alignment in package body
+ "package p;\n"
+ "localparam foo = 4'b0000;\n"
+ "localparam barr = 4'b0010;\n"
+ "localparam baaaaz = 4'b0111;\n"
+ "endpackage\n",
+ "package p;\n"
+ " localparam foo = 4'b0000;\n"
+ " localparam barr = 4'b0010;\n"
+ " localparam baaaaz = 4'b0111;\n"
+ "endpackage\n"},
+ {// parameter alignment in module body
+ "module m;\n"
+ "parameter int foo = 1;\n"
+ "parameter int barr = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter int foo = 1;\n"
+ " parameter int barr = 2;\n"
+ "endmodule\n"},
+ {// parameter alignment in package body
+ "package p;\n"
+ "parameter int foo = 1;\n"
+ "parameter int barrrr = 2;\n"
+ "endpackage\n",
+ "package p;\n"
+ " parameter int foo = 1;\n"
+ " parameter int barrrr = 2;\n"
+ "endpackage\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ 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;
+ }
+}
+
+// Verify kFlushLeft behavior: body-level param/localparam declarations
+// are not aligned.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentFlushLeft) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// module body: parameters are not aligned
+ "module m;\n"
+ "localparam foo = 4'b0000;\n"
+ "localparam barr = 4'b0010;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 4'b0000;\n"
+ " localparam barr = 4'b0010;\n"
+ "endmodule\n"},
+ {// package context: flush-left
+ "package p;\n"
+ "localparam foo = 4'b0000;\n"
+ "localparam barr = 4'b0010;\n"
+ "endpackage\n",
+ "package p;\n"
+ " localparam foo = 4'b0000;\n"
+ " localparam barr = 4'b0010;\n"
+ "endpackage\n"},
+ {// generate context: flush-left
+ "module m;\n"
+ "generate\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "endgenerate\n"
+ "endmodule\n",
+ "module m;\n"
+ " generate\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " endgenerate\n"
+ "endmodule\n"},
+ {// parameter in module body: flush-left
+ "module m;\n"
+ "parameter int W = 8;\n"
+ "parameter int HHHH = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter int W = 8;\n"
+ " parameter int HHHH = 2;\n"
+ "endmodule\n"},
+ {// mixed parameter and localparam: flush-left, no alignment
+ "module m;\n"
+ "parameter int W = 8;\n"
+ "localparam L = 4;\n"
+ "parameter int H = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter int W = 8;\n"
+ " localparam L = 4;\n"
+ " parameter int H = 2;\n"
+ "endmodule\n"},
+ {// interface context: flush-left
+ "interface my_if;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "endinterface\n",
+ "interface my_if;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ "endinterface\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kFlushLeft;
+ 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;
+ }
+}
+
+// Verify kPreserve behavior: body-level param/localparam declarations
+// maintain their existing spacing.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentPreserve) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// module body: existing spacing is kept
+ "module m;\n"
+ "localparam foo = 4'b0000;\n"
+ "localparam barr = 4'b0010;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 4'b0000;\n"
+ " localparam barr = 4'b0010;\n"
+ "endmodule\n"},
+ {// package: existing flush-left spacing preserved
+ "package p;\n"
+ "localparam foo = 4'b0000;\n"
+ "localparam barr = 4'b0010;\n"
+ "endpackage\n",
+ "package p;\n"
+ " localparam foo = 4'b0000;\n"
+ " localparam barr = 4'b0010;\n"
+ "endpackage\n"},
+ {// generate: existing aligned spacing preserved
+ "module m;\n"
+ "generate\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "endgenerate\n"
+ "endmodule\n",
+ "module m;\n"
+ " generate\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " endgenerate\n"
+ "endmodule\n"},
+ {// parameter flush-left preserved
+ "module m;\n"
+ "parameter int W = 8;\n"
+ "parameter int HHHH = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter int W = 8;\n"
+ " parameter int HHHH = 2;\n"
+ "endmodule\n"},
+ {// parameter pre-aligned preserved
+ "module m;\n"
+ "parameter int W = 8;\n"
+ "parameter int HHHH = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter int W = 8;\n"
+ " parameter int HHHH = 2;\n"
+ "endmodule\n"},
+ {// interface pre-aligned preserved
+ "interface my_if;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "endinterface\n",
+ "interface my_if;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ "endinterface\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kPreserve;
+ 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;
+ }
+}
+
+// Verify kAlign behavior for param/localparam declarations across
+// interface, generate, and module body contexts, including mixed
+// param/localparam and packed dimension alignment.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentContexts) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// interface body localparam alignment
+ "interface my_if;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "endinterface\n",
+ "interface my_if;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ "endinterface\n"},
+ {// interface body parameter alignment
+ "interface my_if;\n"
+ "parameter int X = 1;\n"
+ "parameter int Y_LONG = 2;\n"
+ "endinterface\n",
+ "interface my_if;\n"
+ " parameter int X = 1;\n"
+ " parameter int Y_LONG = 2;\n"
+ "endinterface\n"},
+ {// generate block localparam alignment
+ "module m;\n"
+ "generate\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "endgenerate\n"
+ "endmodule\n",
+ "module m;\n"
+ " generate\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " endgenerate\n"
+ "endmodule\n"},
+ {// generate block parameter alignment
+ "module m;\n"
+ "generate\n"
+ "parameter int A = 1;\n"
+ "parameter int BB = 2;\n"
+ "endgenerate\n"
+ "endmodule\n",
+ "module m;\n"
+ " generate\n"
+ " parameter int A = 1;\n"
+ " parameter int BB = 2;\n"
+ " endgenerate\n"
+ "endmodule\n"},
+ {// mixed param and net declarations form separate alignment groups
+ "module m;\n"
+ "localparam X = 1;\n"
+ "localparam YYY = 2;\n"
+ "logic clk;\n"
+ "logic rst_n;\n"
+ "localparam ZZ = 3;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam X = 1;\n"
+ " localparam YYY = 2;\n"
+ " logic clk;\n"
+ " logic rst_n;\n"
+ " localparam ZZ = 3;\n"
+ "endmodule\n"},
+ {// mixed parameter and localparam in same block
+ "module m;\n"
+ "parameter int W = 8;\n"
+ "localparam L = 4;\n"
+ "parameter int H = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter int W = 8;\n"
+ " localparam L = 4;\n"
+ " parameter int H = 2;\n"
+ "endmodule\n"},
+ {// packed dimensions are aligned
+ "module m;\n"
+ "parameter bit [7:0] X = 0;\n"
+ "parameter bit [31:0] YYYY = 1;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter bit [ 7:0] X = 0;\n"
+ " parameter bit [31:0] YYYY = 1;\n"
+ "endmodule\n"},
+ {// multi-identifier comma-separated params are not split (no-crash)
+ "module m;\n"
+ "parameter int a=1, b=2, ccc=3;\n"
+ "localparam d=4, e=5;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter int a = 1, b = 2, ccc = 3;\n"
+ " localparam d = 4, e = 5;\n"
+ "endmodule\n"},
+ {// parameter type declarations do not crash
+ "module m;\n"
+ "parameter type T = int;\n"
+ "parameter type TT_LONG = bit;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter type T = int;\n"
+ " parameter type TT_LONG = bit;\n"
+ "endmodule\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ 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;
+ }
+}
+
+// Verify kInferUserIntent behavior: body-level param/localparam
+// declarations infer alignment intent from existing spacing.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentInferUserIntent) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// flush-left localparams with small spacing diff: infer aligns
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ "endmodule\n"},
+ {// pre-aligned localparams: infer preserves alignment
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "localparam baaaaz = 3;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " localparam baaaaz = 3;\n"
+ "endmodule\n"},
+ {// flush-left params in package with small spacing diff: infer aligns
+ "package p;\n"
+ "localparam X = 1;\n"
+ "localparam YY = 2;\n"
+ "endpackage\n",
+ "package p;\n"
+ " localparam X = 1;\n"
+ " localparam YY = 2;\n"
+ "endpackage\n"},
+ {// mixed param/localparam flush-left: infer keeps flush-left
+ "module m;\n"
+ "parameter int W = 8;\n"
+ "localparam L = 4;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter int W = 8;\n"
+ " localparam L = 4;\n"
+ "endmodule\n"},
+ {// interface flush-left with small diff: infer aligns
+ "interface my_if;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "endinterface\n",
+ "interface my_if;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ "endinterface\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kInferUserIntent;
+ 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;
+ }
+}
+
+// Verify boundary behavior for param declarations with
+// kBlankLinesAndSeparatorComments: both blank lines and separator comments
+// break alignment groups.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentBoundary) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// separator comment breaks param alignment group
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "// ============\n"
+ "localparam baaaaz = 1;\n"
+ "localparam c = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " // ============\n"
+ " localparam baaaaz = 1;\n"
+ " localparam c = 2;\n"
+ "endmodule\n"},
+ {// blank line breaks param alignment group
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "\n"
+ "localparam baaaaz = 1;\n"
+ "localparam c = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ "\n"
+ " localparam baaaaz = 1;\n"
+ " localparam c = 2;\n"
+ "endmodule\n"},
+ {// no separator: single alignment group (all aligned together)
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "localparam baaaaz = 1;\n"
+ "localparam c = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " localparam baaaaz = 1;\n"
+ " localparam c = 2;\n"
+ "endmodule\n"},
+ {// regular comment does NOT break alignment group
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "// regular comment\n"
+ "localparam barr = 2;\n"
+ "localparam baaaaz = 1;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " // regular comment\n"
+ " localparam barr = 2;\n"
+ " localparam baaaaz = 1;\n"
+ "endmodule\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ style.alignment_group_boundary =
+ AlignmentGroupBoundary::kBlankLinesAndSeparatorComments;
+ 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;
+ }
+}
+
+// Verify edge cases: unpacked dimensions, signed/unsigned, implicit types,
+// complex types, single declarations, deeply nested generate blocks.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentEdgeCases) {
+ // Use column_limit = 80 to avoid unintended line wrapping inside
+ // deeply nested generate blocks (generate-if, generate-for), which
+ // would interfere with verifying alignment behavior.
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// unpacked dimension alignment
+ "module m;\n"
+ "parameter bit X [7:0] = 0;\n"
+ "parameter bit YYYY [15:0] = 1;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter bit X [ 7:0] = 0;\n"
+ " parameter bit YYYY[15:0] = 1;\n"
+ "endmodule\n"},
+ {// param declarations with only type and default value (no idim)
+ "module m;\n"
+ "parameter int X = 0;\n"
+ "parameter int YYYY = 0;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter int X = 0;\n"
+ " parameter int YYYY = 0;\n"
+ "endmodule\n"},
+ {// signed modifier with packed dimensions
+ "module m;\n"
+ "parameter bit signed [7:0] X = 0;\n"
+ "parameter bit signed [31:0] YYYY = 1;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter bit signed [ 7:0] X = 0;\n"
+ " parameter bit signed [31:0] YYYY = 1;\n"
+ "endmodule\n"},
+ {// param with complex type (struct/typedef) — does not crash
+ "module m;\n"
+ "parameter foo_t X = foo_t'(0);\n"
+ "parameter foo_t Y_LONG = foo_t'(0);\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter foo_t X = foo_t'(0);\n"
+ " parameter foo_t Y_LONG = foo_t'(0);\n"
+ "endmodule\n"},
+ {// single param declaration (not enough for alignment group)
+ "module m;\n"
+ "localparam X = 1;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam X = 1;\n"
+ "endmodule\n"},
+ {// params inside generate-if block
+ "module m;\n"
+ "generate\n"
+ "if (1) begin : blk\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "end\n"
+ "endgenerate\n"
+ "endmodule\n",
+ "module m;\n"
+ " generate\n"
+ " if (1) begin : blk\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " end\n"
+ " endgenerate\n"
+ "endmodule\n"},
+ {// unsigned keyword with packed dimensions (complement to signed)
+ "module m;\n"
+ "parameter bit unsigned [7:0] X = 0;\n"
+ "parameter bit unsigned [31:0] YYYY = 1;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter bit unsigned [ 7:0] X = 0;\n"
+ " parameter bit unsigned [31:0] YYYY = 1;\n"
+ "endmodule\n"},
+ {// parameter with implicit type (no explicit type keyword)
+ "module m;\n"
+ "parameter X = 32;\n"
+ "parameter Y_LONG = 64;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter X = 32;\n"
+ " parameter Y_LONG = 64;\n"
+ "endmodule\n"},
+ {// params inside generate-for block
+ "module m;\n"
+ "generate\n"
+ "for (i = 0; i < 2; i++) begin : blk\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "end\n"
+ "endgenerate\n"
+ "endmodule\n",
+ "module m;\n"
+ " generate\n"
+ " for (i = 0; i < 2; i++) begin : blk\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " end\n"
+ " endgenerate\n"
+ "endmodule\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 80;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ 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;
+ }
+}
+
+// Verify parameter declarations inside generate-case blocks are aligned.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentGenerateCase) {
+ // Params inside generate-case need wider column limit to avoid wrapping the
+ // case label / begin block line.
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// params inside generate-case block
+ "module m #(P = 0);\n"
+ "generate\n"
+ "case (P)\n"
+ "0: begin : blk\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "end\n"
+ "endcase\n"
+ "endgenerate\n"
+ "endmodule\n",
+ "module m #(\n"
+ " P = 0\n"
+ ");\n"
+ " generate\n"
+ " case (P)\n"
+ " 0: begin : blk\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " end\n"
+ " endcase\n"
+ " endgenerate\n"
+ "endmodule\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 80;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ 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;
+ }
+}
+
+TEST(FormatterEndToEndTest, FormalAndBodyParamAlignmentIndependence) {
+ // Verify that --formal_parameters_alignment and
+ // --parameter_declaration_alignment act independently: setting formal params
+ // to kFlushLeft should not affect body-level param alignment, and vice versa.
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// formal flush-left + body align: only body params align
+ "module m #(\n"
+ "int W = 2,\n"
+ "int LLLL = 4\n"
+ ");\n"
+ "localparam foo = 0;\n"
+ "localparam barrrr = 0;\n"
+ "endmodule\n",
+ "module m #(\n"
+ " int W = 2,\n"
+ " int LLLL = 4\n"
+ ");\n"
+ " localparam foo = 0;\n"
+ " localparam barrrr = 0;\n"
+ "endmodule\n"},
+ {// formal align + body flush-left: only formal params align
+ "module m #(\n"
+ "int W = 2,\n"
+ "int LLLL = 4\n"
+ ");\n"
+ "localparam foo = 0;\n"
+ "localparam barrrr = 0;\n"
+ "endmodule\n",
+ "module m #(\n"
+ " int W = 2,\n"
+ " int LLLL = 4\n"
+ ");\n"
+ " localparam foo = 0;\n"
+ " localparam barrrr = 0;\n"
+ "endmodule\n"},
+ {// both flush-left: no alignment anywhere
+ "module m #(\n"
+ "int W = 2,\n"
+ "int LLLL = 4\n"
+ ");\n"
+ "localparam foo = 0;\n"
+ "localparam barrrr = 0;\n"
+ "endmodule\n",
+ "module m #(\n"
+ " int W = 2,\n"
+ " int LLLL = 4\n"
+ ");\n"
+ " localparam foo = 0;\n"
+ " localparam barrrr = 0;\n"
+ "endmodule\n"},
+ {// both align: both formal and body params aligned
+ "module m #(\n"
+ "int W = 2,\n"
+ "int LLLL = 4\n"
+ ");\n"
+ "localparam foo = 0;\n"
+ "localparam barrrr = 0;\n"
+ "endmodule\n",
+ "module m #(\n"
+ " int W = 2,\n"
+ " int LLLL = 4\n"
+ ");\n"
+ " localparam foo = 0;\n"
+ " localparam barrrr = 0;\n"
+ "endmodule\n"},
+ };
+ // First case: formal kFlushLeft, body kAlign
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ style.formal_parameters_alignment = AlignmentPolicy::kFlushLeft;
+ const auto &tc = kTestCases[0];
+ VLOG(1) << "code-to-format:\n" << tc.input << "<EOF>";
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+ // Second case: formal kAlign, body kFlushLeft
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kFlushLeft;
+ style.formal_parameters_alignment = AlignmentPolicy::kAlign;
+ const auto &tc = kTestCases[1];
+ VLOG(1) << "code-to-format:\n" << tc.input << "<EOF>";
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+ // Third case: both kFlushLeft
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kFlushLeft;
+ style.formal_parameters_alignment = AlignmentPolicy::kFlushLeft;
+ const auto &tc = kTestCases[2];
+ VLOG(1) << "code-to-format:\n" << tc.input << "<EOF>";
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+ // Fourth case: both kAlign
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ style.formal_parameters_alignment = AlignmentPolicy::kAlign;
+ const auto &tc = kTestCases[3];
+ VLOG(1) << "code-to-format:\n" << tc.input << "<EOF>";
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+}
+
+TEST(FormatterEndToEndTest, ClassBodyParamNotAffected) {
+ // Verify that class body parameter/localparam declarations are NOT affected
+ // by --parameter_declaration_alignment, which targets only module/generate/
+ // package/interface body-level params.
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// class body params are not affected
+ "class c;\n"
+ "parameter int X = 1;\n"
+ "parameter int YYYY = 2;\n"
+ "localparam foo = 3;\n"
+ "localparam barrrr = 4;\n"
+ "endclass\n",
+ "class c;\n"
+ " parameter int X = 1;\n"
+ " parameter int YYYY = 2;\n"
+ " localparam foo = 3;\n"
+ " localparam barrrr = 4;\n"
+ "endclass\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ 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;
+ }
+}
+
+TEST(FormatterEndToEndTest, PackageBodyVariableAlignment) {
+ // Verify that moving kPackageItemList to kTabularAlignment in tree-unwrapper
+ // enables net/variable declaration alignment in package bodies via
+ // module_net_variable_alignment.
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// net/variable declaration alignment in package body
+ "package p;\n"
+ "logic [7:0] a;\n"
+ "logic [31:0] bb;\n"
+ "endpackage\n",
+ "package p;\n"
+ " logic [ 7:0] a;\n"
+ " logic [31:0] bb;\n"
+ "endpackage\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.module_net_variable_alignment = AlignmentPolicy::kAlign;
+ 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;
+ }
+}
+
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentApplyToAll) {
+ // Verify that ApplyToAllAlignmentPolicies affects
+ // parameter_declaration_alignment for all four alignment policies.
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// kAlign aligns identifiers and = signs across declarations
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barrrr = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barrrr = 2;\n"
+ "endmodule\n"},
+ {// kFlushLeft: declarations remain flush-left
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barrrr = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barrrr = 2;\n"
+ "endmodule\n"},
+ {// kPreserve: existing spacing is kept
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barrrr = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barrrr = 2;\n"
+ "endmodule\n"},
+ {// kInferUserIntent: large identifier width difference (3 cols)
+ // suggests flush-left intent, no alignment.
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barrrr = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barrrr = 2;\n"
+ "endmodule\n"},
+ };
+ // kAlign
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.ApplyToAllAlignmentPolicies(AlignmentPolicy::kAlign);
+ const auto &tc = kTestCases[0];
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+ // kFlushLeft
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.ApplyToAllAlignmentPolicies(AlignmentPolicy::kFlushLeft);
+ const auto &tc = kTestCases[1];
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+ // kPreserve
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.ApplyToAllAlignmentPolicies(AlignmentPolicy::kPreserve);
+ const auto &tc = kTestCases[2];
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+ // kInferUserIntent
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.ApplyToAllAlignmentPolicies(AlignmentPolicy::kInferUserIntent);
+ const auto &tc = kTestCases[3];
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+}
+
+// Verify that alignment_group_boundary=kBlankLines treats only blank lines
+// (not separator comments) as boundary breaks for param declarations.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentBoundaryBlankLines) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// Only blank line breaks group; separator comment does not
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "\n"
+ "localparam baaaaz = 1;\n"
+ "localparam c = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ "\n"
+ " localparam baaaaz = 1;\n"
+ " localparam c = 2;\n"
+ "endmodule\n"},
+ {// Separator comment does NOT break group with kBlankLines
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "// ============\n"
+ "localparam baaaaz = 1;\n"
+ "localparam c = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " // ============\n"
+ " localparam baaaaz = 1;\n"
+ " localparam c = 2;\n"
+ "endmodule\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ style.alignment_group_boundary = AlignmentGroupBoundary::kBlankLines;
+ 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;
+ }
+}
+
+// Verify that alignment_group_boundary=kNone keeps all param declarations
+// in a single alignment group regardless of blank lines or comments.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentBoundaryNone) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// Blank line does NOT break group with kNone
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "\n"
+ "localparam baaaaz = 1;\n"
+ "localparam c = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ "\n"
+ " localparam baaaaz = 1;\n"
+ " localparam c = 2;\n"
+ "endmodule\n"},
+ {// Separator comment does NOT break group with kNone
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "// ============\n"
+ "localparam baaaaz = 1;\n"
+ "localparam c = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " // ============\n"
+ " localparam baaaaz = 1;\n"
+ " localparam c = 2;\n"
+ "endmodule\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ style.alignment_group_boundary = AlignmentGroupBoundary::kNone;
+ 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;
+ }
+}
+
+// Verify that alignment_group_boundary=kSeparatorComments treats only
+// separator comments (not blank lines) as boundary breaks for param
+// declarations.
+TEST(FormatterEndToEndTest,
+ ParamDeclarationAlignmentBoundarySeparatorComments) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// Separator comment breaks alignment group
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "// ============\n"
+ "localparam baaaaz = 1;\n"
+ "localparam c = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " // ============\n"
+ " localparam baaaaz = 1;\n"
+ " localparam c = 2;\n"
+ "endmodule\n"},
+ {// Blank line does NOT break group with kSeparatorComments
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "\n"
+ "localparam baaaaz = 1;\n"
+ "localparam c = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ "\n"
+ " localparam baaaaz = 1;\n"
+ " localparam c = 2;\n"
+ "endmodule\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ style.alignment_group_boundary = AlignmentGroupBoundary::kSeparatorComments;
+ 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;
+ }
+}
+
+// Verify that class body parameter/localparam declarations are NOT affected
+// by --parameter_declaration_alignment for the kFlushLeft, kPreserve,
+// and kInferUserIntent policies (kAlign is covered in
+// ClassBodyParamNotAffected).
+TEST(FormatterEndToEndTest, ClassBodyParamNotAffectedOtherPolicies) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// kFlushLeft: class body params remain flush-left
+ "class c;\n"
+ "parameter int X = 1;\n"
+ "parameter int YYYY = 2;\n"
+ "localparam foo = 3;\n"
+ "localparam barrrr = 4;\n"
+ "endclass\n",
+ "class c;\n"
+ " parameter int X = 1;\n"
+ " parameter int YYYY = 2;\n"
+ " localparam foo = 3;\n"
+ " localparam barrrr = 4;\n"
+ "endclass\n"},
+ {// kPreserve: parameter_declaration_alignment does not apply to class
+ // bodies; whitespace is normalized by default formatting.
+ "class c;\n"
+ "parameter int X = 1;\n"
+ "parameter int YYYY = 2;\n"
+ "endclass\n",
+ "class c;\n"
+ " parameter int X = 1;\n"
+ " parameter int YYYY = 2;\n"
+ "endclass\n"},
+ {// kInferUserIntent: class body params infer flush-left
+ "class c;\n"
+ "parameter int X = 1;\n"
+ "parameter int YYYY = 2;\n"
+ "endclass\n",
+ "class c;\n"
+ " parameter int X = 1;\n"
+ " parameter int YYYY = 2;\n"
+ "endclass\n"},
+ };
+ // kFlushLeft
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kFlushLeft;
+ const auto &tc = kTestCases[0];
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+ // kPreserve
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kPreserve;
+ const auto &tc = kTestCases[1];
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+ // kInferUserIntent
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kInferUserIntent;
+ const auto &tc = kTestCases[2];
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+}
+
+// Verify formal_parameters_alignment and parameter_declaration_alignment
+// independence for kPreserve and kInferUserIntent combinations.
+TEST(FormatterEndToEndTest,
+ FormalAndBodyParamAlignmentIndependenceOtherPolicies) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// formal kPreserve + body kAlign: only body params aligned
+ "module m #(\n"
+ "int W = 2,\n"
+ "int LLLL = 4\n"
+ ");\n"
+ "localparam foo = 0;\n"
+ "localparam barrrr = 0;\n"
+ "endmodule\n",
+ "module m #(\n"
+ " int W = 2,\n"
+ " int LLLL = 4\n"
+ ");\n"
+ " localparam foo = 0;\n"
+ " localparam barrrr = 0;\n"
+ "endmodule\n"},
+ {// formal kAlign + body kPreserve: only formal params aligned
+ "module m #(\n"
+ "int W = 2,\n"
+ "int LLLL = 4\n"
+ ");\n"
+ "localparam foo = 0;\n"
+ "localparam barrrr = 0;\n"
+ "endmodule\n",
+ "module m #(\n"
+ " int W = 2,\n"
+ " int LLLL = 4\n"
+ ");\n"
+ " localparam foo = 0;\n"
+ " localparam barrrr = 0;\n"
+ "endmodule\n"},
+ {// formal kInferUserIntent + body kAlign: only body params aligned
+ "module m #(\n"
+ "int W = 2,\n"
+ "int LLLL = 4\n"
+ ");\n"
+ "localparam foo = 0;\n"
+ "localparam barrrr = 0;\n"
+ "endmodule\n",
+ "module m #(\n"
+ " int W = 2,\n"
+ " int LLLL = 4\n"
+ ");\n"
+ " localparam foo = 0;\n"
+ " localparam barrrr = 0;\n"
+ "endmodule\n"},
+ };
+ // formal kPreserve + body kAlign
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ style.formal_parameters_alignment = AlignmentPolicy::kPreserve;
+ const auto &tc = kTestCases[0];
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+ // formal kAlign + body kPreserve
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kPreserve;
+ style.formal_parameters_alignment = AlignmentPolicy::kAlign;
+ const auto &tc = kTestCases[1];
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+ // formal kInferUserIntent + body kAlign
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ style.formal_parameters_alignment = AlignmentPolicy::kInferUserIntent;
+ const auto &tc = kTestCases[2];
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+}
+
+// Verify parameter declarations with string, real, and function-call default
+// value types to exercise the ColumnSchemaScanner on non-standard type tokens.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentTypeVariants) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// parameter real type
+ "module m;\n"
+ "parameter real X = 1.0;\n"
+ "parameter real Y_LONG = 2.0;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter real X = 1.0;\n"
+ " parameter real Y_LONG = 2.0;\n"
+ "endmodule\n"},
+ {// parameter string type
+ "module m;\n"
+ "parameter string s = \"hello\";\n"
+ "parameter string long_s = \"world\";\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter string s = \"hello\";\n"
+ " parameter string long_s = \"world\";\n"
+ "endmodule\n"},
+ {// parameter with function call in default value
+ "module m;\n"
+ "parameter int W = func(1, 2);\n"
+ "parameter int H_LONG = other(3);\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter int W = func(1, 2);\n"
+ " parameter int H_LONG = other(3);\n"
+ "endmodule\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ 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;
+ }
+}
+
+// Verify parameter_declaration_alignment kInferUserIntent in package bodies.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentPackageInfer) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// flush-left params with small diff: infer aligns
+ "package p;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "endpackage\n",
+ "package p;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ "endpackage\n"},
+ {// pre-aligned params: infer preserves
+ "package p;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "localparam baaaaz = 3;\n"
+ "endpackage\n",
+ "package p;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " localparam baaaaz = 3;\n"
+ "endpackage\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kInferUserIntent;
+ 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;
+ }
+}
+
+// Verify parameter_declaration_alignment boundary behavior in package bodies.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentPackageBoundary) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// blank line breaks alignment group in package
+ "package p;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "\n"
+ "localparam baaaaz = 1;\n"
+ "localparam c = 2;\n"
+ "endpackage\n",
+ "package p;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ "\n"
+ " localparam baaaaz = 1;\n"
+ " localparam c = 2;\n"
+ "endpackage\n"},
+ {// separator comment breaks alignment group in package
+ "package p;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "// ============\n"
+ "localparam baaaaz = 1;\n"
+ "localparam c = 2;\n"
+ "endpackage\n",
+ "package p;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " // ============\n"
+ " localparam baaaaz = 1;\n"
+ " localparam c = 2;\n"
+ "endpackage\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ style.alignment_group_boundary =
+ AlignmentGroupBoundary::kBlankLinesAndSeparatorComments;
+ 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;
+ }
+}
+
+// Verify the most common default scenario: both formal_parameters_alignment
+// and parameter_declaration_alignment set to kInferUserIntent.
+TEST(FormatterEndToEndTest, BothParamAlignmentsInferUserIntent) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// both flush-left with small diff: infer aligns both formal and body
+ "module m #(\n"
+ "int W = 2,\n"
+ "int LL = 4\n"
+ ");\n"
+ "localparam foo = 0;\n"
+ "localparam barr = 0;\n"
+ "endmodule\n",
+ "module m #(\n"
+ " int W = 2,\n"
+ " int LL = 4\n"
+ ");\n"
+ " localparam foo = 0;\n"
+ " localparam barr = 0;\n"
+ "endmodule\n"},
+ {// both infer: pre-aligned formal, pre-aligned body preserved
+ "module m #(\n"
+ "int W = 2,\n"
+ "int LLLL = 4\n"
+ ");\n"
+ "localparam foo = 0;\n"
+ "localparam barrrr = 0;\n"
+ "endmodule\n",
+ "module m #(\n"
+ " int W = 2,\n"
+ " int LLLL = 4\n"
+ ");\n"
+ " localparam foo = 0;\n"
+ " localparam barrrr = 0;\n"
+ "endmodule\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kInferUserIntent;
+ style.formal_parameters_alignment = AlignmentPolicy::kInferUserIntent;
+ 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;
+ }
+}
+
+// Verify that a large multi-line comment block between parameter
+// declarations does not crash the formatter. The tree-unwrapper may
+// produce a partition structure where the comment tokens are absorbed
+// into the following parameter partition, which the alignment code
+// must handle gracefully by skipping alignment for that row.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentCommentBlockNoCrash) {
+ // These inputs contain large multi-line comment blocks between
+ // parameter/localparam declarations. The formatter must not crash
+ // (SIGABRT). Output correctness is secondary; the formatter may
+ // fall back to preserving the original input when the partition
+ // structure prevents safe alignment.
+ const char *kInputs[] = {
+ "module m;\n"
+ "parameter int W = 8;\n"
+ "// Multi-line comment block\n"
+ "// that spans across\n"
+ "//\n"
+ "// several lines\n"
+ "// of explanatory text\n"
+ "//\n"
+ "// It contains enough\n"
+ "// lines to trigger\n"
+ "// the partition structure\n"
+ "// edge case where\n"
+ "// comment tokens are\n"
+ "// absorbed into the\n"
+ "// following parameter\n"
+ "// partition.\n"
+ "//\n"
+ "localparam int H = 2;\n"
+ "endmodule\n",
+ "package p;\n"
+ "parameter int X = 1;\n"
+ "// Long comment block\n"
+ "// with many lines\n"
+ "//\n"
+ "// of text\n"
+ "//\n"
+ "localparam int Y = 2;\n"
+ "endpackage\n",
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ for (const char *input : kInputs) {
+ VLOG(1) << "code-to-format:\n" << input << "<EOF>";
+ std::ostringstream stream;
+ const auto status = FormatVerilog(input, "<filename>", style, stream);
+ // The primary requirement is that the formatter does not crash
+ // (SIGABRT). Gtest will report failure if the process aborts.
+ // The partition structure may cause output format differences;
+ // the important thing is the formatter handled it gracefully.
+ }
+}
+
+// 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/formatting/token-annotator.cc b/verible/verilog/formatting/token-annotator.cc
index d4af24a..ebaeb21 100644
--- a/verible/verilog/formatting/token-annotator.cc
+++ b/verible/verilog/formatting/token-annotator.cc
@@ -258,6 +258,16 @@
return {0, "No additional space around empty-string tokens."};
}
+ // A macro definition body that begins with the token-concatenation
+ // operator "``" is part of macro name; preserve spacing if present.
+ // If a closing ')', that ends the definition name.
+ if (left.TokenEnum() == verilog_tokentype::PP_Identifier &&
+ right.TokenEnum() == verilog_tokentype::PP_define_body &&
+ right.Text().substr(0, 2) == "``" &&
+ right.OriginalLeadingSpaces().empty()) {
+ return {0, "Preserve spacing in concatenated name"};
+ }
+
// Remove any extra spaces between numeric literals' width, base and digits.
// "16'h123, 'h123" instead of "16 'h123", "16'h 123, 'h 123"
if (IsInsideNumericLiteral(left, right)) {
@@ -500,11 +510,25 @@
// This may be controversial or context-dependent, as parameterized
// classes often appear with method calls like:
// type#(params...)::method(...);
+ // If style.class_parameter_space is enabled, a
+ // parameterized type in a typedef keeps the space before '#':
+ // typedef my_class #(.P(P)) my_class_t;
+ // but a package-qualified type does not, matching the existing
+ // "type#(params...)::method(...)" convention:
+ // typedef pkg::my_class#(.P(P)) my_class_t;
+ // Kept as separate IsInsideFirst() calls because MatchesTagAnyOf()
+ // only unrolls up to four tags.
+ const bool inside_unqualified_typedef =
+ style.class_parameter_space &&
+ left_context.IsInsideFirst({NodeEnum::kTypeDeclaration}, {}) &&
+ !left_context.IsInsideFirst({NodeEnum::kQualifiedId}, {});
+
if (left_context.DirectParentIs(NodeEnum::kUnqualifiedId) &&
!left_context.IsInsideFirst(
{NodeEnum::kInstantiationType, NodeEnum::kBindTargetInstance,
NodeEnum::kExtendsList, NodeEnum::kBraceGroup},
- {})) {
+ {}) &&
+ !inside_unqualified_typedef) {
return {0, "No space before # when direct parent is kUnqualifiedId."};
}
return {1, "Spaces before # in most other contexts."};
diff --git a/verible/verilog/formatting/tree-unwrapper.cc b/verible/verilog/formatting/tree-unwrapper.cc
index 15a0082..2286f8c 100644
--- a/verible/verilog/formatting/tree-unwrapper.cc
+++ b/verible/verilog/formatting/tree-unwrapper.cc
@@ -801,8 +801,10 @@
case NodeEnum::kPreprocessorUndef:
case NodeEnum::kTFPortDeclaration:
case NodeEnum::kTypeDeclaration:
+ case NodeEnum::kLetDeclaration:
case NodeEnum::kNetTypeDeclaration:
case NodeEnum::kForwardDeclaration:
+ case NodeEnum::kInterfaceClassMethod:
case NodeEnum::kConstraintDeclaration:
case NodeEnum::kConstraintExpression:
case NodeEnum::kCovergroupDeclaration:
@@ -881,6 +883,7 @@
case NodeEnum::kTaskDeclaration:
case NodeEnum::kClassDeclaration:
case NodeEnum::kClassHeader:
+ case NodeEnum::kInterfaceClassDeclaration:
case NodeEnum::kBegin:
case NodeEnum::kEnd:
// case NodeEnum::kFork: // TODO(fangism): introduce this node enum
@@ -1201,8 +1204,6 @@
// For the following constructs, always expand the view to subpartitions.
// Add a level of indentation.
case NodeEnum::kPackageImportList:
- case NodeEnum::kPackageItemList:
- case NodeEnum::kInterfaceClassDeclaration:
case NodeEnum::kCasePatternItemList:
case NodeEnum::kConstraintBlockItemList:
case NodeEnum::kConstraintExpressionList:
@@ -1312,8 +1313,11 @@
case NodeEnum::kCaseInsideItemList:
case NodeEnum::kGenerateCaseItemList:
case NodeEnum::kClassItems:
+ case NodeEnum::kInterfaceClassItemList:
case NodeEnum::kModuleItemList:
case NodeEnum::kGenerateItemList:
+ // Aligns parameter, net/variable, and assignment declarations in packages.
+ case NodeEnum::kPackageItemList:
case NodeEnum::kDistributionItemList:
case NodeEnum::kEnumNameList:
case NodeEnum::kStructUnionMemberList: {
@@ -1816,6 +1820,14 @@
}
}
+// True if any token in this leaf partition is an EOL comment.
+static bool PartitionContainsEOLComment(const TokenPartitionTree &partition) {
+ for (const auto &token : partition.Value().TokensRange()) {
+ if (token.TokenEnum() == verilog_tokentype::TK_EOL_COMMENT) return true;
+ }
+ return false;
+}
+
static void PushEndIntoElsePartition(TokenPartitionTree *partition_ptr) {
// Then combine 'end' with the following 'else' ...
// Do not flatten, so that if- and else- clauses can make formatting
@@ -1823,6 +1835,15 @@
auto &partition = *partition_ptr;
auto &if_clause_partition = partition.Children().front();
auto *end_partition = &RightmostDescendant(if_clause_partition);
+ // When 'end' carries a trailing EOL comment, 'else' must start on the next
+ // line (see token annotator: comment before else => MustWrap). Merging
+ // end+comment into the else-if header makes fit-else-expand treat the
+ // header as wider than the eventual formatted line, which wraps the
+ // else-if body on re-format and fails convergence (GitHub issue 2540).
+ if (PartitionContainsEOLComment(*end_partition)) {
+ VLOG(4) << "end has EOL comment, skip merge into else";
+ return;
+ }
auto *end_parent = verible::MergeLeafIntoNextLeaf(end_partition);
// if moving leaf results in any singleton partitions, hoist.
if (end_parent != nullptr) {
@@ -2898,7 +2919,7 @@
case NodeEnum::kConstraintBlockItemList: {
HoistOnlyChildPartition(&partition);
- // Alwyas expand constraint(s) blocks with braces inside them
+ // Always expand constraint(s) blocks with braces inside them
const auto &uwline = partition.Value();
const auto &ftokens = uwline.TokensRange();
auto found = std::find_if(ftokens.begin(), ftokens.end(),
@@ -2970,10 +2991,15 @@
auto &children = partition.Children();
const auto iter1 = std::find_if(children.begin(), children.end(),
PartitionStartsWithSemicolon);
- CHECK(iter1 != children.end());
+ // An attribute instance ((* ... *)) in the for-loop header can change
+ // the partition structure so that the expected semicolon-leading
+ // partitions are not present. When they are missing, leave the
+ // partitions unreshaped rather than aborting (avoids a CHECK-failure
+ // crash on attributed/unusual for-headers).
+ if (iter1 == children.end()) break;
const auto iter2 =
std::find_if(iter1 + 1, children.end(), PartitionStartsWithSemicolon);
- CHECK(iter2 != children.end());
+ if (iter2 == children.end()) break;
const int dist1 = std::distance(children.begin(), iter1);
const int dist2 = std::distance(children.begin(), iter2);
VLOG(4) << "kForSpec got ';' at child " << dist1 << " and " << dist2;
diff --git a/verible/verilog/parser/verilog-parser_test.cc b/verible/verilog/parser/verilog-parser_test.cc
index 7c99a1e..754498b 100644
--- a/verible/verilog/parser/verilog-parser_test.cc
+++ b/verible/verilog/parser/verilog-parser_test.cc
@@ -1339,6 +1339,15 @@
static constexpr ParserTestCaseArray kModuleTests = {
"module modular_thing;\n"
"endmodule",
+ "module m;\n"
+ "wire logic b;\n"
+ "endmodule",
+ "module m;\n"
+ "wire bit b;\n"
+ "endmodule",
+ "module m;\n"
+ "wire logic [3:0] b;\n"
+ "endmodule",
"module semicolon_madness;;;;;\n"
"endmodule",
"module automatic modular_thing;\n"
@@ -1381,6 +1390,62 @@
"input logic [N:0] a;\n"
"output logic co;\n"
"endmodule",
+ "module t;\n"
+ "input bit i_bit;\n"
+ "input byte i_byte;\n"
+ "input chandle i_chandle;\n"
+ "input event i_event;\n"
+ "input int i_int;\n"
+ "input integer i_inte;\n"
+ "input longint i_longint;\n"
+ "input real i_real;\n"
+ "input realtime i_realtime;\n"
+ "input shortint i_shortint;\n"
+ "input shortreal i_shortreal;\n"
+ "input string i_string;\n"
+ "input time i_time;\n"
+ "output bit o_bit;\n"
+ "output byte o_byte;\n"
+ "output chandle o_chandle;\n"
+ "output event o_event;\n"
+ "output int o_int;\n"
+ "output integer o_inte;\n"
+ "output longint o_longint;\n"
+ "output real o_real;\n"
+ "output realtime o_realtime;\n"
+ "output shortint o_shortint;\n"
+ "output shortreal o_shortreal;\n"
+ "output string o_string;\n"
+ "output time o_time;\n"
+ "endmodule\n",
+ "module t (\n"
+ "input bit i_bit,\n"
+ "input byte i_byte,\n"
+ "input chandle i_chandle,\n"
+ "input event i_event,\n"
+ "input int i_int,\n"
+ "input integer i_inte,\n"
+ "input longint i_longint,\n"
+ "input real i_real,\n"
+ "input realtime i_realtime,\n"
+ "input shortint i_shortint,\n"
+ "input shortreal i_shortreal,\n"
+ "input string i_string,\n"
+ "input time i_time,\n"
+ "output bit o_bit,\n"
+ "output byte o_byte,\n"
+ "output chandle o_chandle,\n"
+ "output event o_event,\n"
+ "output int o_int,\n"
+ "output integer o_inte,\n"
+ "output longint o_longint,\n"
+ "output real o_real,\n"
+ "output realtime o_realtime,\n"
+ "output shortint o_shortint,\n"
+ "output shortreal o_shortreal,\n"
+ "output string o_string,\n"
+ "output time o_time);\n"
+ "endmodule\n",
"module zoom (a, co);\n"
"input bus_type a;\n"
"output bus_type [3:0] co;\n"
@@ -3796,6 +3861,10 @@
"union tagged packed { int i; bit b; } foo;",
"union tagged packed signed { int i; bit b; } foo;",
"union tagged packed unsigned { int i; bit b; } foo;",
+ "union soft { int i; bit b; } foo;",
+ "union soft packed { int i; bit b; } foo;",
+ "union soft packed signed { int i; bit b; } foo;",
+ "union soft packed unsigned { int i; bit b; } foo;",
};
// TODO(fangism): implement and test ENUM_CONSTANT
diff --git a/verible/verilog/parser/verilog.y b/verible/verilog/parser/verilog.y
index 6a4a6f5..a6de1a8 100644
--- a/verible/verilog/parser/verilog.y
+++ b/verible/verilog/parser/verilog.y
@@ -679,7 +679,7 @@
/* most likely a lexical error */
%token TK_OTHER
-// LINT.ThenChange(../formatting/verilog_token.cc)
+// LINT.ThenChange(../formatting/verilog-token.cc)
/* A glorified ';' specialized to mark the end of an
assertion_variable_declaration list inside the
@@ -2111,6 +2111,9 @@
{ $$ = MakeTaggedNode(N::kDataTypeImplicitIdDimensions,
MakeDataType($1, MakePackedDimensionsNode($2)),
$3, nullptr, nullptr); }
+ | data_type_primitive delay3_or_drive_opt
+ { $$ = MakeTaggedNode(N::kDataTypeImplicitIdDimensions,
+ $1, $2, nullptr, nullptr); }
| GenericIdentifier decl_dimensions_opt delay3_or_drive_opt
{ $$ = MakeTaggedNode(N::kDataTypeImplicitIdDimensions,
MakeDataType(MakeTaggedNode(N::kLocalRoot,MakeTaggedNode(N::kUnqualifiedId,$1)), MakePackedDimensionsNode($2)),
@@ -3726,7 +3729,7 @@
struct_data_type
: TK_struct packed_signing_opt '{' struct_union_member_list '}'
{ $$ = MakeTaggedNode(N::kStructType, $1, $2, MakeBraceGroup($3, $4, $5)); }
- | TK_union TK_tagged_opt packed_signing_opt '{' struct_union_member_list '}'
+ | TK_union TK_union_qualifier_opt packed_signing_opt '{' struct_union_member_list '}'
{ $$ = MakeTaggedNode(N::kUnionType, $1, $2, $3,
MakeBraceGroup($4, $5, $6)); }
;
@@ -5620,16 +5623,55 @@
| 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 ';'
+ { $$ = MakeTaggedNode(N::kModulePortDeclaration, $1,
+ MakeDataType(MakeTaggedNode(N::kDataTypePrimitive, $2, $3),
+ MakePackedDimensionsNode($4)),
+ $5, $6); }
+ | port_direction integer_atom_type signed_unsigned_opt
+ list_of_identifiers_unpacked_dimensions ';'
+ { $$ = MakeTaggedNode(N::kModulePortDeclaration, $1,
+ MakeDataType(MakeTaggedNode(N::kDataTypePrimitive, $2, $3)),
+ $4, $5); }
+ | port_direction non_integer_type
+ list_of_identifiers_unpacked_dimensions ';'
+ { $$ = MakeTaggedNode(N::kModulePortDeclaration, $1,
+ MakeDataType(MakeTaggedNode(N::kDataTypePrimitive, $2)),
+ $3, $4); }
+ | port_direction TK_string
+ list_of_identifiers_unpacked_dimensions ';'
+ { $$ = MakeTaggedNode(N::kModulePortDeclaration, $1,
+ MakeDataType(MakeTaggedNode(N::kDataTypePrimitive, $2)),
+ $3, $4); }
+ | port_direction TK_event
+ list_of_identifiers_unpacked_dimensions ';'
+ { $$ = MakeTaggedNode(N::kModulePortDeclaration, $1,
+ MakeDataType(MakeTaggedNode(N::kDataTypePrimitive, $2)),
+ $3, $4); }
+ | port_direction TK_chandle
+ list_of_identifiers_unpacked_dimensions ';'
+ { $$ = MakeTaggedNode(N::kModulePortDeclaration, $1,
+ MakeDataType(MakeTaggedNode(N::kDataTypePrimitive, $2)),
+ $3, $4); }
;
parameter_override
@@ -7375,8 +7417,10 @@
| /* empty */
{ $$ = nullptr; }
;
-TK_tagged_opt
- : TK_tagged
+TK_union_qualifier_opt
+ : TK_soft
+ { $$ = std::move($1); }
+ | TK_tagged
{ $$ = std::move($1); }
| /* empty */
{ $$ = nullptr; }
diff --git a/verible/verilog/preprocessor/verilog-preprocess.h b/verible/verilog/preprocessor/verilog-preprocess.h
index 568ed5e..2d134ae 100644
--- a/verible/verilog/preprocessor/verilog-preprocess.h
+++ b/verible/verilog/preprocessor/verilog-preprocess.h
@@ -124,7 +124,7 @@
// Initialize preprocessing with safe default options
// TODO(hzeller): remove this constructor once all places using the
// preprocessor have been updated to pass a config.
- VerilogPreprocess() : VerilogPreprocess(Config()){};
+ VerilogPreprocess() : VerilogPreprocess(Config()) {};
// ScanStream reads in a stream of tokens returns the result as a move
// of preprocessor_data_. preprocessor_data_ should not be accessed
diff --git a/verible/verilog/tools/formatter/README.md b/verible/verilog/tools/formatter/README.md
index 0dbafdb..a3ffb45 100644
--- a/verible/verilog/tools/formatter/README.md
+++ b/verible/verilog/tools/formatter/README.md
@@ -4,8 +4,8 @@
freshness: { owner: 'hzeller' reviewed: '2020-10-07' }
*-->
-`verible-verilog-format` is the SystemVerilog formatter tool. You can can
-get a full set of avilable flags using the `--helpfull` flag.
+`verible-verilog-format` is the SystemVerilog formatter tool. You can
+get a full set of available flags using the `--helpfull` flag.
For automatic formatting suggestions on github pull requests, there is a
[easy to integrate github action available][github-format-action].
@@ -35,12 +35,18 @@
operator.); default: 4;
Flags from verilog/formatting/format_style_init.cc:
- --assignment_statement_alignment (Format various assignments:
+ --assignment_statement_alignment (Format various assignments in
+ module, generate, interface, and package bodies:
{align,flush-left,preserve,infer}); default: infer;
--case_items_alignment (Format case items:
{align,flush-left,preserve,infer}); default: infer;
--class_member_variable_alignment (Format class member variables:
{align,flush-left,preserve,infer}); default: infer;
+ --class_parameter_space (If true, keep/insert a space
+ before '#' in a class parameterized typedef, e.g. "typedef
+ my_class #(.P(P)) my_class_t;". If false (default), no space is
+ inserted, matching the IEEE convention used for parameterized class
+ instantiations, e.g. "type#(params...)::method(...)".); default: false;
--compact_indexing_and_selections (Use compact binary expressions inside
indexing / bit selection operators); default: true;
--distribution_items_alignment (Align distribution items:
@@ -48,11 +54,13 @@
--enum_assignment_statement_alignment (Format assignments with enums:
{align,flush-left,preserve,infer}); default: infer;
--expand_coverpoints (If true, always expand coverpoints.); default: false;
- --formal_parameters_alignment (Format formal parameters:
- {align,flush-left,preserve,infer}); default: infer;
+ --formal_parameters_alignment (Format formal parameters in module/
+ interface/class headers (inside #(...)): {align,flush-left,preserve,infer});
+ default: infer;
--formal_parameters_indentation (Indent formal parameters: {indent,wrap});
default: wrap;
- --module_net_variable_alignment (Format net/variable declarations:
+ --module_net_variable_alignment (Format net/variable declarations
+ in module, generate, interface, and package bodies:
{align,flush-left,preserve,infer}); default: infer;
--named_parameter_alignment (Format named actual parameters:
{align,flush-left,preserve,infer}); default: infer;
@@ -62,6 +70,10 @@
{align,flush-left,preserve,infer}); default: infer;
--named_port_indentation (Indent named port connections: {indent,wrap});
default: wrap;
+ --parameter_declaration_alignment (Format parameter/localparam declarations
+ in module, generate, interface, and package bodies:
+ {align,flush-left,preserve,infer}); default: infer;
+ NOTE: class body parameter declarations are NOT affected.
--port_declarations_alignment (Format port declarations:
{align,flush-left,preserve,infer}); default: infer;
--port_declarations_indentation (Indent port declarations: {indent,wrap});
diff --git a/verible/verilog/tools/kythe/README.md b/verible/verilog/tools/kythe/README.md
index b44aaf9..610f755 100644
--- a/verible/verilog/tools/kythe/README.md
+++ b/verible/verilog/tools/kythe/README.md
@@ -30,4 +30,6 @@
File search will stop at the the first found among the listed directories.
e.g --include_dir_paths directory1,directory2
if "A.sv" exists in both "directory1" and "directory2" the one in "directory1" is the one we will use)
+ --output_path (Path of the file to write the extracted Kythe facts to.
+ Writes to stdout if empty or "-"); default: "";
```
diff --git a/verible/verilog/tools/kythe/kythe-facts.cc b/verible/verilog/tools/kythe/kythe-facts.cc
index 3880d91..948a54e 100644
--- a/verible/verilog/tools/kythe/kythe-facts.cc
+++ b/verible/verilog/tools/kythe/kythe-facts.cc
@@ -154,12 +154,16 @@
stream << idt << "\"source\": ";
source_node.FormatJSON(stream, debug, indent_more) << "," << separator;
}
- { stream << idt << "\"edge_kind\": \"" << edge_name << "\"," << separator; }
+ {
+ stream << idt << "\"edge_kind\": \"" << edge_name << "\"," << separator;
+ }
{
stream << idt << "\"target\": ";
target_node.FormatJSON(stream, debug, indent_more) << "," << separator;
}
- { stream << idt << "\"fact_name\": \"/\"" << separator; }
+ {
+ stream << idt << "\"fact_name\": \"/\"" << separator;
+ }
return stream << verible::Spacer(indentation) << "}";
}
diff --git a/verible/verilog/tools/kythe/kythe-proto-output.cc b/verible/verilog/tools/kythe/kythe-proto-output.cc
index 0117329..76e5952 100644
--- a/verible/verilog/tools/kythe/kythe-proto-output.cc
+++ b/verible/verilog/tools/kythe/kythe-proto-output.cc
@@ -14,9 +14,12 @@
#include "verible/verilog/tools/kythe/kythe-proto-output.h"
+#include <memory>
+#include <ostream>
#include <string>
#include "google/protobuf/io/coded_stream.h"
+#include "google/protobuf/io/zero_copy_stream.h"
#include "google/protobuf/io/zero_copy_stream_impl.h"
#include "third_party/proto/kythe/storage.pb.h"
#include "verible/verilog/tools/kythe/kythe-facts.h"
@@ -26,7 +29,8 @@
namespace {
using ::google::protobuf::io::CodedOutputStream;
-using ::google::protobuf::io::FileOutputStream;
+using ::google::protobuf::io::OstreamOutputStream;
+using ::google::protobuf::io::ZeroCopyOutputStream;
using ::kythe::proto::Entry;
// Returns the VName representation in Kythe's storage proto format.
@@ -60,7 +64,7 @@
}
// Output entry to the stream.
-void OutputProto(const Entry &entry, FileOutputStream *stream) {
+void OutputProto(const Entry &entry, ZeroCopyOutputStream *stream) {
CodedOutputStream coded_stream(stream);
coded_stream.WriteVarint32(entry.ByteSizeLong());
entry.SerializeToCodedStream(&coded_stream);
@@ -68,14 +72,17 @@
} // namespace
-KytheProtoOutput::KytheProtoOutput(int fd) : out_(fd) {}
-KytheProtoOutput::~KytheProtoOutput() { out_.Close(); }
+KytheProtoOutput::KytheProtoOutput(std::ostream &output_stream)
+ : out_(std::make_unique<OstreamOutputStream>(&output_stream)) {}
+
+// OstreamOutputStream flushes its remaining bytes when destroyed.
+KytheProtoOutput::~KytheProtoOutput() = default;
void KytheProtoOutput::Emit(const Fact &fact) {
- OutputProto(ConvertFactToEntry(fact), &out_);
+ OutputProto(ConvertFactToEntry(fact), out_.get());
}
void KytheProtoOutput::Emit(const Edge &edge) {
- OutputProto(ConvertEdgeToEntry(edge), &out_);
+ OutputProto(ConvertEdgeToEntry(edge), out_.get());
}
} // namespace kythe
diff --git a/verible/verilog/tools/kythe/kythe-proto-output.h b/verible/verilog/tools/kythe/kythe-proto-output.h
index ba873cd..2ffa58e 100644
--- a/verible/verilog/tools/kythe/kythe-proto-output.h
+++ b/verible/verilog/tools/kythe/kythe-proto-output.h
@@ -15,7 +15,10 @@
#ifndef VERIBLE_VERILOG_TOOLS_KYTHE_KYTHE_PROTO_OUTPUT_H_
#define VERIBLE_VERILOG_TOOLS_KYTHE_KYTHE_PROTO_OUTPUT_H_
-#include "google/protobuf/io/zero_copy_stream_impl.h"
+#include <memory>
+#include <ostream>
+
+#include "google/protobuf/io/zero_copy_stream.h"
#include "verible/verilog/tools/kythe/kythe-facts-extractor.h"
#include "verible/verilog/tools/kythe/kythe-facts.h"
@@ -24,7 +27,10 @@
class KytheProtoOutput final : public KytheOutput {
public:
- explicit KytheProtoOutput(int output_fd);
+ // Writes to an already-open stream. The stream must outlive this object and
+ // must be opened in binary mode, as the proto entries are not text.
+ explicit KytheProtoOutput(std::ostream &output_stream);
+
~KytheProtoOutput() final;
// Output Kythe facts from the indexing data in proto format.
@@ -32,7 +38,7 @@
void Emit(const Edge &edge) final;
private:
- ::google::protobuf::io::FileOutputStream out_;
+ std::unique_ptr<::google::protobuf::io::ZeroCopyOutputStream> out_;
};
} // namespace kythe
diff --git a/verible/verilog/tools/kythe/verilog-kythe-extractor.cc b/verible/verilog/tools/kythe/verilog-kythe-extractor.cc
index df43e52..321db49 100644
--- a/verible/verilog/tools/kythe/verilog-kythe-extractor.cc
+++ b/verible/verilog/tools/kythe/verilog-kythe-extractor.cc
@@ -12,7 +12,11 @@
// See the License for the specific language governing permissions and
// limitations under the License.
+#include <fstream>
+#include <ios>
#include <iostream>
+#include <memory>
+#include <ostream>
#include <sstream>
#include <string>
#include <string_view>
@@ -33,11 +37,9 @@
#include "verible/verilog/tools/kythe/kythe-facts.h"
#include "verible/verilog/tools/kythe/kythe-proto-output.h"
-#ifndef _WIN32
-#include <unistd.h> // for STDOUT_FILENO
-#else
-#include <stdio.h>
-#define STDOUT_FILENO _fileno(stdout)
+#ifdef _WIN32
+#include <fcntl.h>
+#include <io.h>
#endif
// for --print_kythe_facts flag
@@ -109,14 +111,18 @@
ABSL_FLAG(std::string, verilog_project_name, "",
"Verilog project name to use as Kythe corpus. Optional");
+ABSL_FLAG(std::string, output_path, "",
+ R"(Path of the file to write the extracted Kythe facts to.
+Writes to stdout if empty or "-".)");
+
namespace verilog {
namespace kythe {
-// Prints Kythe facts in proto format to stdout.
+// Prints Kythe facts in proto format to the given stream.
static void PrintKytheFactsProtoEntries(
const IndexingFactNode &file_list_facts_tree, const VerilogProject &project,
- int fd) {
- KytheProtoOutput proto_output(fd);
+ std::ostream &stream) {
+ KytheProtoOutput proto_output(stream);
StreamKytheFactsEntries(&proto_output, file_list_facts_tree, project);
}
@@ -134,7 +140,7 @@
static std::vector<absl::Status> ExtractTranslationUnits(
std::string_view file_list_path, VerilogProject *project,
- const std::vector<std::string> &file_names) {
+ const std::vector<std::string> &file_names, std::ostream &output_stream) {
std::vector<absl::Status> errors;
const verilog::kythe::IndexingFactNode file_list_facts_tree(
verilog::kythe::ExtractFiles(file_list_path, project, file_names,
@@ -149,17 +155,17 @@
// check how to output kythe facts.
switch (absl::GetFlag(FLAGS_print_kythe_facts)) {
case PrintMode::kJSON:
- std::cout << KytheFactsPrinter(file_list_facts_tree, *project)
- << std::endl;
+ output_stream << KytheFactsPrinter(file_list_facts_tree, *project)
+ << std::endl;
break;
case PrintMode::kJSONDebug:
- std::cout << KytheFactsPrinter(file_list_facts_tree, *project,
- /*debug=*/true)
- << std::endl;
+ output_stream << KytheFactsPrinter(file_list_facts_tree, *project,
+ /*debug=*/true)
+ << std::endl;
break;
case PrintMode::kProto:
PrintKytheFactsProtoEntries(file_list_facts_tree, *project,
- STDOUT_FILENO);
+ output_stream);
break;
case PrintMode::kNone:
KytheFactsNullPrinter(file_list_facts_tree, *project);
@@ -173,6 +179,13 @@
} // namespace verilog
int main(int argc, char **argv) {
+#ifdef _WIN32
+ // Windows messes with newlines by default. Fix this here, so that stdout
+ // carries the same bytes as --output_path, and so that the proto entries
+ // stay binary.
+ _setmode(_fileno(stdout), _O_BINARY);
+#endif
+
const auto usage =
absl::StrCat("usage: ", argv[0], " [options] --file_list_path FILE\n", R"(
Extracts kythe indexing facts from the given SystemVerilog source files.
@@ -211,9 +224,24 @@
absl::GetFlag(FLAGS_verilog_project_name),
/*provide_lookup_file_origin=*/false);
+ // Send the facts to a file when asked to, otherwise to stdout. Both go out
+ // in binary mode, because --print_kythe_facts=proto is not text.
+ const std::string output_path = absl::GetFlag(FLAGS_output_path);
+ std::unique_ptr<std::ofstream> file_closer;
+ std::ostream *output_stream = &std::cout;
+ if (!output_path.empty() && output_path != "-") {
+ file_closer = std::make_unique<std::ofstream>(
+ output_path, std::ios::out | std::ios::binary);
+ if (!file_closer->good()) {
+ LOG(ERROR) << "Failed to create/open output file: " << output_path;
+ return 1;
+ }
+ output_stream = file_closer.get();
+ }
+
const std::vector<absl::Status> errors(
verilog::kythe::ExtractTranslationUnits(file_list_path, &project,
- file_paths));
+ file_paths, *output_stream));
if (!errors.empty()) {
LOG(ERROR) << "Encountered some issues while indexing files (could result "
"in missing indexing data):"
diff --git a/verible/verilog/tools/kythe/verilog_kythe_extractor_test.sh b/verible/verilog/tools/kythe/verilog_kythe_extractor_test.sh
index bf3bd7c..1e7bcc9 100755
--- a/verible/verilog/tools/kythe/verilog_kythe_extractor_test.sh
+++ b/verible/verilog/tools/kythe/verilog_kythe_extractor_test.sh
@@ -160,4 +160,90 @@
}
################################################################################
+echo "=== Write facts to --output_path instead of stdout."
+
+cat > "$MY_INPUT_FILE" <<EOF
+localparam int fooo = 1;
+localparam int barr = fooo;
+EOF
+
+echo "myinput.txt" > "${TEST_TMPDIR}/file_list"
+
+# Every print mode has to honor --output_path, and has to write the same bytes
+# it would have written to stdout.
+for mode in json json_debug proto ; do
+ "$extractor" \
+ --file_list_path "${TEST_TMPDIR}/file_list" \
+ --file_list_root "$(dirname "$MY_INPUT_FILE")" \
+ --print_kythe_facts="$mode" \
+ --output_path "$MY_OUTPUT_FILE" 2>/dev/null
+
+ status="$?"
+ [[ $status == 0 ]] || {
+ echo "Expected exit code 0 for --print_kythe_facts=$mode, but got $status"
+ exit 1
+ }
+
+ [[ -s "$MY_OUTPUT_FILE" ]] || {
+ echo "Expected --output_path file to be non-empty for mode $mode."
+ exit 1
+ }
+
+ "$extractor" \
+ --file_list_path "${TEST_TMPDIR}/file_list" \
+ --file_list_root "$(dirname "$MY_INPUT_FILE")" \
+ --print_kythe_facts="$mode" \
+ > "$MY_EXPECT_FILE" 2>/dev/null
+
+ cmp "$MY_OUTPUT_FILE" "$MY_EXPECT_FILE" || {
+ echo "--output_path output differs from stdout output for mode $mode."
+ exit 1
+ }
+done
+
+################################################################################
+echo "=== '--output_path -' writes to stdout."
+
+"$extractor" \
+ --file_list_path "${TEST_TMPDIR}/file_list" \
+ --file_list_root "$(dirname "$MY_INPUT_FILE")" \
+ --print_kythe_facts=json \
+ --output_path - \
+ > "$MY_OUTPUT_FILE" 2>/dev/null
+
+status="$?"
+[[ $status == 0 ]] || {
+ echo "Expected exit code 0, but got $status"
+ exit 1
+}
+
+grep -q "signature" "$MY_OUTPUT_FILE" || {
+ echo "Expected \"signature\" in $MY_OUTPUT_FILE but didn't find it. Got:"
+ cat "$MY_OUTPUT_FILE"
+ exit 1
+}
+
+################################################################################
+echo "=== Expect failure on unwritable --output_path."
+
+"$extractor" \
+ --file_list_path "${TEST_TMPDIR}/file_list" \
+ --file_list_root "$(dirname "$MY_INPUT_FILE")" \
+ --print_kythe_facts=json \
+ --output_path "${TEST_TMPDIR}/nonexistent-dir/out.json" \
+ > "$MY_OUTPUT_FILE" 2>&1
+
+status="$?"
+[[ $status == 1 ]] || {
+ echo "Expected exit code 1, but got $status"
+ exit 1
+}
+
+grep -q "Failed to create/open output file" "$MY_OUTPUT_FILE" || {
+ echo "Expected \"Failed to create/open output file\" in $MY_OUTPUT_FILE but didn't find it. Got:"
+ cat "$MY_OUTPUT_FILE"
+ exit 1
+}
+
+################################################################################
echo "PASS"
diff --git a/verible/verilog/tools/lint/README.md b/verible/verilog/tools/lint/README.md
index a91128d..f1117be 100644
--- a/verible/verilog/tools/lint/README.md
+++ b/verible/verilog/tools/lint/README.md
@@ -33,7 +33,7 @@
## Developers
-[Style lint rule development guide](../../../doc/style_lint.md).
+[Style lint rule development guide](../../../../doc/style_lint.md).
## Usage
diff --git a/verible/verilog/tools/ls/autoexpand.cc b/verible/verilog/tools/ls/autoexpand.cc
index 0f9d5c4..82f7de0 100644
--- a/verible/verilog/tools/ls/autoexpand.cc
+++ b/verible/verilog/tools/ls/autoexpand.cc
@@ -1464,10 +1464,26 @@
module->RetrieveDependencies(modules_);
}
// Sort modules in the buffer based on a dependency graph, so that AUTOs are
- // expanded in order
+ // expanded in order. Counting dependents and breaking ties by name induces a
+ // strict weak ordering even with dependency loops or independent modules.
+ absl::flat_hash_map<const Module *, int> dependents_count;
+ for (const Module *module : buffer_modules) {
+ int count = 0;
+ for (const Module *other : buffer_modules) {
+ if (other != module && other->DependsOn(module)) {
+ ++count;
+ }
+ }
+ dependents_count[module] = count;
+ }
std::sort(buffer_modules.begin(), buffer_modules.end(),
- [](const Module *left, const Module *right) {
- return right->DependsOn(left);
+ [&dependents_count](const Module *left, const Module *right) {
+ const int left_count = dependents_count.at(left);
+ const int right_count = dependents_count.at(right);
+ if (left_count != right_count) {
+ return left_count > right_count;
+ }
+ return left->Name() < right->Name();
});
for (Module *const module : buffer_modules) {
// Ports declared in AUTOINPUT/AUTOINOUT/AUTOOUTPUT must be removed from
