Merge origin/master into fix/2544 Resolve formatter_test.cc conflict by keeping MacroBeforeCloseParenFormatEquivalent alongside master's newer parameter-declaration alignment tests.
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..3c1c724 100755 --- a/.github/bin/run-format.sh +++ b/.github/bin/run-format.sh
@@ -47,12 +47,6 @@ 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-test.sh b/.github/bin/smoke-test.sh index ebe5679..ba3ff96 100755 --- a/.github/bin/smoke-test.sh +++ b/.github/bin/smoke-test.sh
@@ -135,14 +135,14 @@ ExpectedFailCount[syntax:ibex]=13 ExpectedFailCount[lint:ibex]=13 -ExpectedFailCount[project:ibex]=223 +ExpectedFailCount[project:ibex]=218 ExpectedFailCount[preprocessor:ibex]=397 -ExpectedFailCount[syntax:opentitan]=92 -ExpectedFailCount[lint:opentitan]=92 -ExpectedFailCount[project:opentitan]=1179 +ExpectedFailCount[syntax:opentitan]=88 +ExpectedFailCount[lint:opentitan]=88 +ExpectedFailCount[project:opentitan]=1068 ExpectedFailCount[formatter:opentitan]=0 -ExpectedFailCount[preprocessor:opentitan]=3061 +ExpectedFailCount[preprocessor:opentitan]=3074 ExpectedFailCount[syntax:sv-tests]=74 ExpectedFailCount[lint:sv-tests]=73 @@ -151,8 +151,8 @@ ExpectedFailCount[syntax:caliptra-rtl]=39 ExpectedFailCount[lint:caliptra-rtl]=38 -ExpectedFailCount[project:caliptra-rtl]=455 -ExpectedFailCount[preprocessor:caliptra-rtl]=903 +ExpectedFailCount[project:caliptra-rtl]=453 +ExpectedFailCount[preprocessor:caliptra-rtl]=911 ExpectedFailCount[syntax:Cores-VeeR-EH2]=2 ExpectedFailCount[lint:Cores-VeeR-EH2]=2 @@ -161,17 +161,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[project:uvm]=40 ExpectedFailCount[preprocessor:uvm]=126 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 +187,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]=117 +ExpectedFailCount[lint:ivtest]=117 +ExpectedFailCount[project:ivtest]=146 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 +205,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
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..5f03f2f 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 @@ -36,13 +36,15 @@ - 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 +126,6 @@ - test - test-clang - test-nortti - - test-c++20 - test-c++23 - smoke-test #- smoke-test-analyzer #issue: #2046 @@ -141,8 +142,6 @@ exclude: - mode: test-nortti arch: arm64 - - mode: test-c++20 - arch: arm64 - mode: test-c++23 arch: arm64 - mode: asan @@ -182,12 +181,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 +330,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 +393,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 +424,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..b813b76 100644 --- a/README.md +++ b/README.md
@@ -172,10 +172,8 @@ 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 (Min version 7) 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. @@ -216,17 +214,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
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/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/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/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..e15ec5f 100644 --- a/verible/verilog/analysis/symbol-table_test.cc +++ b/verible/verilog/analysis/symbol-table_test.cc
@@ -713,6 +713,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/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..2dde818 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}"); @@ -167,7 +175,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 +188,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);
diff --git a/verible/verilog/formatting/format-style.h b/verible/verilog/formatting/format-style.h index e73cf14..2fad329 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; @@ -139,7 +149,7 @@ // 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 +184,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_test.cc b/verible/verilog/formatting/formatter_test.cc index 654cc0a..9c59626 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,19 @@ " .L(L),\n" " .W(W)\n" ") bar_t;\n"}, + // 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 test cases {"package fedex;localparam int www=3 ;endpackage : fedex\n", @@ -8125,6 +8273,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" @@ -19234,6 +19398,1513 @@ EXPECT_THAT(stream.str(), testing::HasSubstr("`TOKEN_BYTE")); } +// 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. + } +} + } // namespace } // namespace formatter } // namespace verilog
diff --git a/verible/verilog/formatting/token-annotator.cc b/verible/verilog/formatting/token-annotator.cc index d4af24a..def1349 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,23 @@ // This may be controversial or context-dependent, as parameterized // classes often appear with method calls like: // type#(params...)::method(...); + // 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 = + 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..9664c98 100644 --- a/verible/verilog/formatting/tree-unwrapper.cc +++ b/verible/verilog/formatting/tree-unwrapper.cc
@@ -803,6 +803,7 @@ case NodeEnum::kTypeDeclaration: case NodeEnum::kNetTypeDeclaration: case NodeEnum::kForwardDeclaration: + case NodeEnum::kInterfaceClassMethod: case NodeEnum::kConstraintDeclaration: case NodeEnum::kConstraintExpression: case NodeEnum::kCovergroupDeclaration: @@ -881,6 +882,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 +1203,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 +1312,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: { @@ -2898,7 +2901,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(),
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..f1c24a8 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)); } ; @@ -5630,6 +5633,37 @@ { $$ = MakeTaggedNode(N::kModulePortDeclaration, $1, MakeDataType($3, ForwardChildren($2), 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 +7409,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..01b2869 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,7 +35,8 @@ 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; @@ -48,11 +49,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 +65,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/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/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