From 78d6ec8114ee3e270b76a4db96920cc3e73333f0 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 4 Jan 2025 12:55:15 -0500 Subject: [PATCH] Fix error message when call task as a function (#3089). --- Changes | 1 + src/V3LinkDot.cpp | 6 -- src/V3Width.cpp | 33 +++++++-- test_regress/t/t_class_func_nvoid_bad.out | 25 +++++++ test_regress/t/t_class_func_nvoid_bad.py | 16 +++++ test_regress/t/t_class_func_nvoid_bad.v | 76 +++++++++++++++++++++ test_regress/t/t_func_task_bad.out | 7 +- test_regress/t/t_func_void_bad.out | 11 +-- test_regress/t/t_func_void_bad.v | 4 ++ test_regress/t/t_lint_implicit_func_bad.out | 3 +- 10 files changed, 165 insertions(+), 17 deletions(-) create mode 100644 test_regress/t/t_class_func_nvoid_bad.out create mode 100755 test_regress/t/t_class_func_nvoid_bad.py create mode 100644 test_regress/t/t_class_func_nvoid_bad.v diff --git a/Changes b/Changes index 815da1703..fc1016325 100644 --- a/Changes +++ b/Changes @@ -13,6 +13,7 @@ Verilator 5.033 devel **Minor:** +* Fix error message when call task as a function (#3089). [Matthew Ballance] * Fix V3Simulate constant reuse (#5709). [Geza Lore] * Fix man pages what-is section (#5710). [Ahmed El-Mahmoudy] * Fix pattern assignment to real inside struct (#5713). diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index c747239f4..8c54b2984 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -2336,11 +2336,6 @@ class LinkDotResolveVisitor final : public VNVisitor { nodep->addStmtsp(superNewStmtp); return superNewStmtp; } - void taskFuncSwapCheck(AstNodeFTaskRef* nodep) { - if (nodep->taskp() && VN_IS(nodep->taskp(), Task) && VN_IS(nodep, FuncRef)) { - nodep->v3error("Illegal call of a task as a function: " << nodep->prettyNameQ()); - } - } void checkNoDot(AstNode* nodep) { if (VL_UNLIKELY(m_ds.m_dotPos != DP_NONE)) { // UINFO(9, indent() << "ds=" << m_ds.ascii() << endl); @@ -3779,7 +3774,6 @@ class LinkDotResolveVisitor final : public VNVisitor { okSymp->cellErrorScopes(nodep); } } - taskFuncSwapCheck(nodep); } } void visit(AstSelBit* nodep) override { diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 0c9a39299..0dca57d26 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -3929,11 +3929,14 @@ class WidthVisitor final : public VNVisitor { AstNodeExpr* argsp = nullptr; if (nodep->pinsp()) argsp = nodep->pinsp()->unlinkFrBackWithNext(); AstNodeFTaskRef* newp = nullptr; - if (VN_IS(ftaskp, Task)) { - newp = new AstTaskRef{nodep->fileline(), VN_AS(ftaskp, Task), argsp}; + // We use m_vup to determine task or function, so that later error checks + // for funcref->task and taskref->func will pick up properly + if (!m_vup) { // Called as task + newp = new AstTaskRef{nodep->fileline(), nodep->name(), argsp}; } else { - newp = new AstFuncRef{nodep->fileline(), VN_AS(ftaskp, Func), argsp}; + newp = new AstFuncRef{nodep->fileline(), nodep->name(), argsp}; } + newp->taskp(ftaskp); // Not passed above as might be func vs task mismatch newp->classOrPackagep(classp); nodep->replaceWith(newp); VL_DO_DANGLING(nodep->deleteTree(), nodep); @@ -3941,7 +3944,18 @@ class WidthVisitor final : public VNVisitor { nodep->taskp(ftaskp); nodep->dtypeFrom(ftaskp); nodep->classOrPackagep(classp); - if (VN_IS(ftaskp, Task)) nodep->dtypeSetVoid(); + if (VN_IS(ftaskp, Task)) { + if (!m_vup) { + nodep->dtypeSetVoid(); + } else { + if (m_vup->prelim()) { + nodep->v3error( + "Cannot call a task/void-function as a member function: " + << nodep->prettyNameQ()); + } + nodep->dtypeSetVoid(); + } + } if (withp) nodep->addPinsp(withp); processFTaskRefArgs(nodep); } @@ -5824,7 +5838,15 @@ class WidthVisitor final : public VNVisitor { } void visit(AstFuncRef* nodep) override { visit(static_cast(nodep)); - nodep->dtypeFrom(nodep->taskp()); + if (nodep->taskp() && VN_IS(nodep->taskp(), Task)) { + UASSERT_OBJ(m_vup, nodep, "Function reference where widthed expression expection"); + if (m_vup->prelim()) + nodep->v3error( + "Cannot call a task/void-function as a function: " << nodep->prettyNameQ()); + nodep->dtypeSetVoid(); + } else { + nodep->dtypeFrom(nodep->taskp()); + } if (nodep->fileline()->timingOn()) { AstNodeModule* const classp = nodep->classOrPackagep(); if (nodep->name() == "self" && classp->name() == "process") { @@ -6089,6 +6111,7 @@ class WidthVisitor final : public VNVisitor { processFTaskRefArgs(nodep); nodep->addPinsp(withp); nodep->didWidth(true); + // See steps that follow in visit(AstFuncRef*) } void visit(AstNodeProcedure* nodep) override { assertAtStatement(nodep); diff --git a/test_regress/t/t_class_func_nvoid_bad.out b/test_regress/t/t_class_func_nvoid_bad.out new file mode 100644 index 000000000..52690482f --- /dev/null +++ b/test_regress/t/t_class_func_nvoid_bad.out @@ -0,0 +1,25 @@ +%Error: t/t_class_func_nvoid_bad.v:47:11: Cannot call a task/void-function as a function: 'mod_fv' + : ... note: In instance 't' + 47 | if (mod_fv() == 10) $stop; + | ^~~~~~ +%Error: t/t_class_func_nvoid_bad.v:50:11: Cannot call a task/void-function as a function: 'mod_t' + : ... note: In instance 't' + 50 | if (mod_t() == 10) $stop; + | ^~~~~ +%Error: t/t_class_func_nvoid_bad.v:58:13: Cannot call a task/void-function as a member function: 'fv' + : ... note: In instance 't' + 58 | if (c.fv() == 10) $stop; + | ^~ +%Error: t/t_class_func_nvoid_bad.v:61:13: Cannot call a task/void-function as a member function: 't' + : ... note: In instance 't' + 61 | if (c.t() == 10) $stop; + | ^ +%Error: t/t_class_func_nvoid_bad.v:69:13: Cannot call a task/void-function as a function: 'sfv' + : ... note: In instance 't' + 69 | if (c.sfv() == 10) $stop; + | ^~~ +%Error: t/t_class_func_nvoid_bad.v:72:13: Cannot call a task/void-function as a function: 'st' + : ... note: In instance 't' + 72 | if (c.st() == 10) $stop; + | ^~ +%Error: Exiting due to diff --git a/test_regress/t/t_class_func_nvoid_bad.py b/test_regress/t/t_class_func_nvoid_bad.py new file mode 100755 index 000000000..31228c9a7 --- /dev/null +++ b/test_regress/t/t_class_func_nvoid_bad.py @@ -0,0 +1,16 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2024 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 + +import vltest_bootstrap + +test.scenarios('linter') + +test.lint(fails=True, expect_filename=test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_class_func_nvoid_bad.v b/test_regress/t/t_class_func_nvoid_bad.v new file mode 100644 index 000000000..f8180e69b --- /dev/null +++ b/test_regress/t/t_class_func_nvoid_bad.v @@ -0,0 +1,76 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2020 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + + +class Cls; + function int fi(); + return 10; + endfunction + function void fv(); + endfunction + task t(); + endtask + static function int sfi(); + return 10; + endfunction + static function void sfv(); + endfunction + static task st(); + endtask +endclass + +module t (/*AUTOARG*/); + + function int mod_fi(); + return 10; + endfunction + function void mod_fv(); + endfunction + task mod_t(); + endtask + + initial begin + Cls c; + c = new; + + // For test of calling function in void context, see t_func_void_bad.v + + // Module + if (mod_fi() != 10) $stop; // OK + void'(mod_fi()); // OK + + mod_fv(); // Warn IGNOREDRETURN + void'(mod_fv()); // OK + if (mod_fv() == 10) $stop; // Bad call of task as function + + mod_t(); // OK + if (mod_t() == 10) $stop; // Bad call of task as function + + // Member functions + if (c.fi() != 10) $stop; // OK + void'(c.fi()); // OK + + c.fv(); // Ok + void'(c.fv()); // OK + if (c.fv() == 10) $stop; // Bad + + c.t(); // OK + if (c.t() == 10) $stop; // Bad + + // Static member functions + if (c.sfi() != 10) $stop; // OK + void'(c.sfi()); // OK + + c.sfv(); // Ok + void'(c.sfv()); // OK + if (c.sfv() == 10) $stop; // Bad + + c.st(); // OK + if (c.st() == 10) $stop; // Bad + + $stop; + end +endmodule diff --git a/test_regress/t/t_func_task_bad.out b/test_regress/t/t_func_task_bad.out index 5a85eb04d..3cd03ead0 100644 --- a/test_regress/t/t_func_task_bad.out +++ b/test_regress/t/t_func_task_bad.out @@ -1,4 +1,9 @@ -%Error: t/t_func_task_bad.v:10:11: Illegal call of a task as a function: 'task_as_func' +%Error: t/t_func_task_bad.v:10:11: Cannot call a task/void-function as a function: 'task_as_func' + : ... note: In instance 't' 10 | if (task_as_func(1'b0)) $stop; | ^~~~~~~~~~~~ +%Error: t/t_func_task_bad.v:10:7: Logical operator IF expects a non-complex data type on the If. + : ... note: In instance 't' + 10 | if (task_as_func(1'b0)) $stop; + | ^~ %Error: Exiting due to diff --git a/test_regress/t/t_func_void_bad.out b/test_regress/t/t_func_void_bad.out index e14223a08..37b0f4cb6 100644 --- a/test_regress/t/t_func_void_bad.out +++ b/test_regress/t/t_func_void_bad.out @@ -1,9 +1,12 @@ -%Warning-IGNOREDRETURN: t/t_func_void_bad.v:21:7: Ignoring return value of non-void function (IEEE 1800-2023 13.4.1) - 21 | f1(); +%Warning-IGNOREDRETURN: t/t_func_void_bad.v:24:7: Ignoring return value of non-void function (IEEE 1800-2023 13.4.1) + 24 | f1(); | ^~ ... For warning description see https://verilator.org/warn/IGNOREDRETURN?v=latest ... Use "/* verilator lint_off IGNOREDRETURN */" and lint_on around source to disable this message. -%Warning-IGNOREDRETURN: t/t_func_void_bad.v:24:9: Ignoring return value of non-void function (IEEE 1800-2023 13.4.1) - 24 | c.fi(); +%Warning-IGNOREDRETURN: t/t_func_void_bad.v:27:9: Ignoring return value of non-void function (IEEE 1800-2023 13.4.1) + 27 | c.fi(); | ^~ +%Warning-IGNOREDRETURN: t/t_func_void_bad.v:28:9: Ignoring return value of non-void function (IEEE 1800-2023 13.4.1) + 28 | c.sfi(); + | ^~~ %Error: Exiting due to diff --git a/test_regress/t/t_func_void_bad.v b/test_regress/t/t_func_void_bad.v index 837595812..e9552e58c 100644 --- a/test_regress/t/t_func_void_bad.v +++ b/test_regress/t/t_func_void_bad.v @@ -8,6 +8,9 @@ class Cls; function int fi(); return 10; endfunction + static function int sfi(); + return 10; + endfunction endclass module t; @@ -22,6 +25,7 @@ module t; // c = new; c.fi(); // Bad - ignored result + c.sfi(); // Bad - ignored result // $write("*-* All Finished *-*\n"); $finish; diff --git a/test_regress/t/t_lint_implicit_func_bad.out b/test_regress/t/t_lint_implicit_func_bad.out index 6d790a04c..78301f0c4 100644 --- a/test_regress/t/t_lint_implicit_func_bad.out +++ b/test_regress/t/t_lint_implicit_func_bad.out @@ -1,4 +1,5 @@ -%Error: t/t_lint_implicit_func_bad.v:12:11: Illegal call of a task as a function: 'imp_func_conflict' +%Error: t/t_lint_implicit_func_bad.v:12:11: Cannot call a task/void-function as a function: 'imp_func_conflict' + : ... note: In instance 't' 12 | assign imp_func_conflict = 1'b1; | ^~~~~~~~~~~~~~~~~ %Error: Exiting due to