From 472ad90d837448ebc8e1647d54bbc4fa5bed681a Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Mon, 9 Oct 2023 04:12:01 -0400 Subject: [PATCH] Change lint_off to not propagate upwards to files including where the lint_off is. --- Changes | 1 + docs/guide/warnings.rst | 28 +++++++++++++------- src/V3ParseImp.cpp | 22 ++++++++++----- test_regress/t/t_lint_warn_incfile2_bad.out | 5 ++++ test_regress/t/t_lint_warn_incfile2_bad.pl | 22 +++++++++++++++ test_regress/t/t_lint_warn_incfile2_bad.v | 14 ++++++++++ test_regress/t/t_lint_warn_incfile2_bad_b.vh | 10 +++++++ test_regress/t/t_pp_line_bad.out | 4 +-- test_regress/t/t_vlt_warn_file_bad.out | 5 ++++ test_regress/t/t_vlt_warn_file_bad.pl | 22 +++++++++++++++ test_regress/t/t_vlt_warn_file_bad.v | 12 +++++++++ test_regress/t/t_vlt_warn_file_bad.vlt | 10 +++++++ test_regress/t/t_vlt_warn_file_bad_b.vh | 9 +++++++ 13 files changed, 145 insertions(+), 19 deletions(-) create mode 100644 test_regress/t/t_lint_warn_incfile2_bad.out create mode 100755 test_regress/t/t_lint_warn_incfile2_bad.pl create mode 100644 test_regress/t/t_lint_warn_incfile2_bad.v create mode 100644 test_regress/t/t_lint_warn_incfile2_bad_b.vh create mode 100644 test_regress/t/t_vlt_warn_file_bad.out create mode 100755 test_regress/t/t_vlt_warn_file_bad.pl create mode 100644 test_regress/t/t_vlt_warn_file_bad.v create mode 100644 test_regress/t/t_vlt_warn_file_bad.vlt create mode 100644 test_regress/t/t_vlt_warn_file_bad_b.vh diff --git a/Changes b/Changes index 64c4f6832..dc07f520b 100644 --- a/Changes +++ b/Changes @@ -18,6 +18,7 @@ Verilator 5.017 devel * Support randc (#4349). * Support resizing function call inout arguments (#4467). * Support converting parameters inside modules to localparams (#4511). [Anthony Donlon] +* Change lint_off to not propagate upwards to files including where the lint_off is. * Fix conversion of impure logical expressions to bit expressions (#487 partial) (#4437). [Ryszard Rozak, Antmicro Ltd.] * Fix enum functions in localparams (#3999). [Andrew Nolte] * Fix passing arguments by reference (#3385 partial) (#4489). [Ryszard Rozak, Antmicro Ltd.] diff --git a/docs/guide/warnings.rst b/docs/guide/warnings.rst index 48c0888f9..4588fd28b 100644 --- a/docs/guide/warnings.rst +++ b/docs/guide/warnings.rst @@ -12,10 +12,18 @@ Disabling Warnings Warnings may be disabled in multiple ways: -#. Disable the warning in the source code. When the warning is printed, it - will include a warning code. Surround the offending line with a - :code:`/*verilator&32;lint_off*/` and :code:`/*verilator&32;lint_on*/` - metacomment pair: +#. Disable the warning globally by invoking Verilator with the + :code:`-Wno-{warning-code}` option. + + Global disables should be avoided, as they removes all checking across + the source files, and prevents other users from compiling the sources + without knowing the magic set of disables needed to compile those + sources successfully. + +#. Disable the warning in the design source code. When the warning is + printed, it will include a warning code. Surround the offending line + with a :code:`/*verilator&32;lint_off*/` and + :code:`/*verilator&32;lint_on*/` metacomment pair: .. code-block:: sv @@ -23,6 +31,12 @@ Warnings may be disabled in multiple ways: if (`DEF_THAT_IS_EQ_ZERO <= 3) $stop; // verilator lint_on UNSIGNED + + A lint_off in the design source code will propagate down to any child + files (files later included by the file with the lint_off), but will not + propagate upwards to any parent file (file that included the file with + the lint_off). + #. Disable the warning using :ref:`Configuration Files` with a :option:`lint_off` command. This is useful when a script suppresses warnings, and the Verilog source should not be changed. This method also @@ -32,12 +46,6 @@ Warnings may be disabled in multiple ways: lint_off -rule UNSIGNED -file "*/example.v" -line 1 -#. Disable the warning globally invoking Verilator with the - :code:`-Wno-{warning-code}` option. This should be avoided, as it - removes all checking across the designs, and prevents other users from - compiling your code without knowing the magic set of disables needed to - compile your design successfully. - Error And Warning Format ======================== diff --git a/src/V3ParseImp.cpp b/src/V3ParseImp.cpp index 0e082d402..cb23e938f 100644 --- a/src/V3ParseImp.cpp +++ b/src/V3ParseImp.cpp @@ -72,15 +72,25 @@ V3ParseImp::~V3ParseImp() { void V3ParseImp::lexPpline(const char* textp) { // Handle lexer `line directive - FileLine* const prevFl = lexFileline()->copyOrSameFileLineApplied(); - int enterExit; - lexFileline()->lineDirective(textp, enterExit /*ref*/); + // FileLine* const prevFl = lexFileline(); + string newFilename; + int newLineno = -1; + int enterExit = 0; + lexFileline()->lineDirectiveParse(textp, newFilename /*ref*/, newLineno /*ref*/, enterExit /*ref*/); if (enterExit == 1) { // Enter + FileLine* const prevFl = lexFileline()->copyOrSameFileLine(); // Without applyIgnores + FileLine* const newFl = new FileLine{prevFl}; // Not copyOrSameFileLine as need to keep old value + lexFileline(newFl); lexFileline()->parent(prevFl); } else if (enterExit == 2) { // Exit - FileLine* upFl = lexFileline()->parent(); - if (upFl) upFl = upFl->parent(); - if (upFl) lexFileline()->parent(upFl); + if (FileLine* upFl = lexFileline()->parent()) { + lexFileline(upFl); // Restore warning state to upper file + } + } + if (enterExit != -1) { // Line/fn change + lexFileline()->filename(newFilename); + lexFileline()->lineno(newLineno); + lexFileline()->applyIgnores(); } } diff --git a/test_regress/t/t_lint_warn_incfile2_bad.out b/test_regress/t/t_lint_warn_incfile2_bad.out new file mode 100644 index 000000000..5060cbd21 --- /dev/null +++ b/test_regress/t/t_lint_warn_incfile2_bad.out @@ -0,0 +1,5 @@ +%Warning-WIDTHTRUNC: t/t_lint_warn_incfile2_bad.v:13:17: Operator ASSIGN expects 32 bits on the Assign RHS, but Assign RHS's CONST '64'h1' generates 64 bits. + : ... note: In instance 't' + ... For warning description see https://verilator.org/warn/WIDTHTRUNC?v=latest + ... Use "/* verilator lint_off WIDTHTRUNC */" and lint_on around source to disable this message. +%Error: Exiting due to diff --git a/test_regress/t/t_lint_warn_incfile2_bad.pl b/test_regress/t/t_lint_warn_incfile2_bad.pl new file mode 100755 index 000000000..b3cdb56a9 --- /dev/null +++ b/test_regress/t/t_lint_warn_incfile2_bad.pl @@ -0,0 +1,22 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2008 by Wilson Snyder. This program is free software; you +# can redistribute it and/or modify it under the terms of either the GNU +# Lesser General Public License Version 3 or the Perl Artistic License +# Version 2.0. +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +scenarios(vlt => 1); + +lint( + # See also t/t_lint_warn_incfile1_bad + # See also t/t_vlt_warn_file_bad + verilator_flags2 => ["--no-std"], + fails => 1, + expect_filename => $Self->{golden_filename}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_lint_warn_incfile2_bad.v b/test_regress/t/t_lint_warn_incfile2_bad.v new file mode 100644 index 000000000..0d821dcd8 --- /dev/null +++ b/test_regress/t/t_lint_warn_incfile2_bad.v @@ -0,0 +1,14 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2023 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +// Check that lint_off doesn't propagate from include, for post-preprocessor warnings + +`include "t_lint_warn_incfile2_bad_b.vh" + +module t; + sub sub(); + int warn_t = 64'h1; // Not suppressed - should warn +endmodule diff --git a/test_regress/t/t_lint_warn_incfile2_bad_b.vh b/test_regress/t/t_lint_warn_incfile2_bad_b.vh new file mode 100644 index 000000000..aae34e35c --- /dev/null +++ b/test_regress/t/t_lint_warn_incfile2_bad_b.vh @@ -0,0 +1,10 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2023 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +module sub; + // verilator lint_off WIDTHTRUNC + int warn_sub = 64'h1; // Suppressed +endmodule diff --git a/test_regress/t/t_pp_line_bad.out b/test_regress/t/t_pp_line_bad.out index fc54a045d..3041d5dfa 100644 --- a/test_regress/t/t_pp_line_bad.out +++ b/test_regress/t/t_pp_line_bad.out @@ -25,7 +25,5 @@ %Error: t/t_pp_line_bad.v:7:1: Define or directive not defined: '`line' 7 | `line | ^~~~~ -%Error: t/t_pp_line_bad.v:15:1: `line was not properly formed with '`line number "filename" level' -%Error: t/t_pp_line_bad.v:17:1: `line was not properly formed with '`line number "filename" level' -%Error: t/t_pp_line_bad.v:19:1: `line was not properly formed with '`line number "filename" level' +%Error: t/t_pp_line_bad.v:14:1: `line was not properly formed with '`line number "filename" level' %Error: Exiting due to diff --git a/test_regress/t/t_vlt_warn_file_bad.out b/test_regress/t/t_vlt_warn_file_bad.out new file mode 100644 index 000000000..6ff3c6fe0 --- /dev/null +++ b/test_regress/t/t_vlt_warn_file_bad.out @@ -0,0 +1,5 @@ +%Warning-WIDTHTRUNC: t/t_vlt_warn_file_bad.v:11:17: Operator ASSIGN expects 32 bits on the Assign RHS, but Assign RHS's CONST '64'h1' generates 64 bits. + : ... note: In instance 't' + ... For warning description see https://verilator.org/warn/WIDTHTRUNC?v=latest + ... Use "/* verilator lint_off WIDTHTRUNC */" and lint_on around source to disable this message. +%Error: Exiting due to diff --git a/test_regress/t/t_vlt_warn_file_bad.pl b/test_regress/t/t_vlt_warn_file_bad.pl new file mode 100755 index 000000000..83d974461 --- /dev/null +++ b/test_regress/t/t_vlt_warn_file_bad.pl @@ -0,0 +1,22 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2008 by Wilson Snyder. This program is free software; you +# can redistribute it and/or modify it under the terms of either the GNU +# Lesser General Public License Version 3 or the Perl Artistic License +# Version 2.0. +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +scenarios(vlt => 1); + +lint( + # See also t/t_lint_warn_incfile1_bad + # See also t/t_lint_warn_incfile2_bad + verilator_flags2 => ["--no-std t/t_vlt_warn_file_bad.vlt"], + fails => 1, + expect_filename => $Self->{golden_filename}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_vlt_warn_file_bad.v b/test_regress/t/t_vlt_warn_file_bad.v new file mode 100644 index 000000000..89ef7bfc2 --- /dev/null +++ b/test_regress/t/t_vlt_warn_file_bad.v @@ -0,0 +1,12 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2023 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +`include "t_vlt_warn_file_bad_b.vh" + +module t; + sub sub(); + int warn_t = 64'h1; +endmodule diff --git a/test_regress/t/t_vlt_warn_file_bad.vlt b/test_regress/t/t_vlt_warn_file_bad.vlt new file mode 100644 index 000000000..14107e8a0 --- /dev/null +++ b/test_regress/t/t_vlt_warn_file_bad.vlt @@ -0,0 +1,10 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2023 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +`verilator_config + +// Test that this -file rule doesn't turn off warnings in t/t_vlt_warn_file_bad.v +lint_off -rule WIDTHTRUNC -file "t/t_vlt_warn_file_bad_b.vh" diff --git a/test_regress/t/t_vlt_warn_file_bad_b.vh b/test_regress/t/t_vlt_warn_file_bad_b.vh new file mode 100644 index 000000000..bf1bb1067 --- /dev/null +++ b/test_regress/t/t_vlt_warn_file_bad_b.vh @@ -0,0 +1,9 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2023 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +module sub; + int warn_sub = 64'h1; +endmodule