Add a compatibility flag to allow implicit call expressions to be rec…

…ursive function calls
MikePopoloski · Nov 21, 2023 · 1a6d089 · 1a6d089
1 parent 2bae995
commit 1a6d089
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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<VariableSymbol>().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<SubroutineSymbol>().returnValVar == symbol) {

diff --git 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<ExpressionStatement>(expr, syntax.sourceRange());
     if (expr.bad())
         return badStmt(compilation, result);

diff --git 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
@@ -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);
+}