From 1a6d089a4ddbafa2b7046d853e9452341760b45e Mon Sep 17 00:00:00 2001 From: MikePopoloski Date: Mon, 20 Nov 2023 20:31:13 -0500 Subject: [PATCH] Add a compatibility flag to allow implicit call expressions to be recursive function calls --- docs/command-line-ref.dox | 6 ++++ include/slang/ast/Compilation.h | 5 +++- source/ast/Expression.cpp | 9 ++++-- source/ast/Statements.cpp | 2 +- source/driver/Driver.cpp | 5 +++- tests/unittests/ast/StatementTests.cpp | 38 ++++++++++++++++++++++++++ 6 files changed, 60 insertions(+), 5 deletions(-) diff --git a/docs/command-line-ref.dox b/docs/command-line-ref.dox index 17e02c665..6afc43e2c 100644 --- a/docs/command-line-ref.dox +++ b/docs/command-line-ref.dox @@ -300,6 +300,12 @@ compatibility purposes. Allow top-level modules to have interface ports. SystemVerilog doesn't permit this, but some tools allow it anyway so this flag can be used for compatibility purposes. +`--allow-recursive-implicit-call` + +Allow implicit call expressions (ones that lack parentheses) to be recursive function calls. +The LRM states that directly recursive function calls require parentheses but many other tools +allow this anyway so this flag can be used for compatibility purposes. + `--strict-driver-checking` Perform strict driver checking, which currently means disabling procedural 'for' @ref loop-unroll diff --git a/include/slang/ast/Compilation.h b/include/slang/ast/Compilation.h index 5fca79256..1b3b56f26 100644 --- a/include/slang/ast/Compilation.h +++ b/include/slang/ast/Compilation.h @@ -110,8 +110,11 @@ enum class SLANG_EXPORT CompilationFlags { /// Allow strings to implicitly convert to integers. RelaxStringConversions = 1 << 9, + + /// Allow implicit call expressions (lacking parentheses) to be recursive function calls. + AllowRecursiveImplicitCall = 1 << 10 }; -SLANG_BITMASK(CompilationFlags, RelaxStringConversions) +SLANG_BITMASK(CompilationFlags, AllowRecursiveImplicitCall) /// Contains various options that can control compilation behavior. struct SLANG_EXPORT CompilationOptions { diff --git a/source/ast/Expression.cpp b/source/ast/Expression.cpp index d134dc077..1011a38cd 100644 --- a/source/ast/Expression.cpp +++ b/source/ast/Expression.cpp @@ -1024,8 +1024,13 @@ Expression& Expression::bindLookupResult(Compilation& compilation, LookupResult& // If we've found a function's return value variable and have parentheses for // a call expression, translate that symbol to be the subroutine itself to // allow for recursive function calls. - if (invocation && symbol->kind == SymbolKind::Variable && result.selectors.empty()) { - if (auto scope = symbol->getParentScope()) { + if (symbol->kind == SymbolKind::Variable && result.selectors.empty() && + symbol->as().flags.has(VariableFlags::CompilerGenerated)) { + + auto scope = symbol->getParentScope(); + if (scope && + (invocation || (context.flags.has(ASTFlags::TopLevelStatement) && + compilation.hasFlag(CompilationFlags::AllowRecursiveImplicitCall)))) { auto& sym = scope->asSymbol(); if (sym.kind == SymbolKind::Subroutine && sym.as().returnValVar == symbol) { diff --git a/source/ast/Statements.cpp b/source/ast/Statements.cpp index b1c71c882..d1a87cc34 100644 --- a/source/ast/Statements.cpp +++ b/source/ast/Statements.cpp @@ -2314,7 +2314,7 @@ Statement& ExpressionStatement::fromSyntax(Compilation& compilation, Statement& ExpressionStatement::fromSyntax(Compilation& compilation, const VoidCastedCallStatementSyntax& syntax, const ASTContext& context) { - auto& expr = Expression::bind(*syntax.expr, context); + auto& expr = Expression::bind(*syntax.expr, context, ASTFlags::TopLevelStatement); auto result = compilation.emplace(expr, syntax.sourceRange()); if (expr.bad()) return badStmt(compilation, result); diff --git a/source/driver/Driver.cpp b/source/driver/Driver.cpp index 3054c7ad1..3db1467d9 100644 --- a/source/driver/Driver.cpp +++ b/source/driver/Driver.cpp @@ -151,6 +151,8 @@ void Driver::addStandardArgs() { "by initial blocks."); addCompFlag(CompilationFlags::AllowTopLevelIfacePorts, "--allow-toplevel-iface-ports", "Allow top-level modules to have interface ports."); + addCompFlag(CompilationFlags::AllowRecursiveImplicitCall, "--allow-recursive-implicit-call", + "Allow implicit call expressions to be recursive function calls."); addCompFlag(CompilationFlags::StrictDriverChecking, "--strict-driver-checking", "Perform strict driver checking, which currently means disabling " "procedural 'for' loop unrolling."); @@ -388,7 +390,8 @@ bool Driver::processOptions() { auto vcsCompatFlags = {CompilationFlags::AllowHierarchicalConst, CompilationFlags::AllowUseBeforeDeclare, CompilationFlags::RelaxEnumConversions, - CompilationFlags::RelaxStringConversions}; + CompilationFlags::RelaxStringConversions, + CompilationFlags::AllowRecursiveImplicitCall}; for (auto flag : vcsCompatFlags) { auto& option = options.compilationFlags.at(flag); diff --git a/tests/unittests/ast/StatementTests.cpp b/tests/unittests/ast/StatementTests.cpp index 1bc537c79..7443cde0e 100644 --- a/tests/unittests/ast/StatementTests.cpp +++ b/tests/unittests/ast/StatementTests.cpp @@ -1934,3 +1934,41 @@ endmodule CHECK(diags[0].code == diag::StatementNotInLoop); CHECK(diags[1].code == diag::StatementNotInLoop); } + +TEST_CASE("Implicit call in recursive function") { + auto tree = SyntaxTree::fromText(R"( +class Packet; + bit [3:0] command; + integer status; + + function integer current_status(); + current_status = status; + endfunction + + int incr = 0; + + function integer update_status(); + update_status = status; + if (incr < 2) begin + incr = incr + 1; + update_status; + end + + if (update_status == 0) + if (this.update_status) + command[update_status] = 1'b0; + return update_status; + endfunction +endclass +)"); + + CompilationOptions options; + options.flags |= CompilationFlags::AllowRecursiveImplicitCall; + + Compilation compilation(options); + compilation.addSyntaxTree(tree); + + auto& diags = compilation.getAllDiagnostics(); + REQUIRE(diags.size() == 1); + CHECK(diags[0].code == diag::UnusedResult); +}