MikePopoloski / slang

SystemVerilog compiler and language services
MIT License
558 stars 122 forks source link

[bug][slang] Incorrect AST of nested submodule #1010

Closed likeamahoney closed 1 month ago

likeamahoney commented 1 month ago

Hi! There is a difference between constructed AST by slang if nested submodule declaration contains the ports or not. For example if we have a nested submodule without declared ports:

module test();
    module test1();
        function void foo();
        endfunction
    endmodule
endmodule

It gives such AST with foo function declared and test1 module instantiated:

{
  "design": {
    "name": "$root",
    "kind": "Root",
    "addr": 2676705606176,
    "members": [
      {
        "name": "",
        "kind": "CompilationUnit",
        "addr": 2676704990432
      },
      {
        "name": "test",
        "kind": "Instance",
        "addr": 2676704991056,
        "body": {
          "name": "test",
          "kind": "InstanceBody",
          "addr": 2676704990728,
          "members": [
            {
              "name": "test1",
              "kind": "Instance",
              "addr": 2676704991576,
              "body": {
                "name": "test1",
                "kind": "InstanceBody",
                "addr": 2676704991696,
                "members": [
                  {
                    "name": "foo",
                    "kind": "Subroutine",
                    "addr": 2676704991952,
                    "members": [
                      {
                        "name": "foo",
                        "kind": "Variable",
                        "addr": 2676704992240,
                        "type": "void",
                        "lifetime": "Automatic",
                        "flags": "compiler_generated"
                      }
                    ],
                    "returnType": "void",
                    "defaultLifetime": "Static",
                    "subroutineKind": "Function",
                    "body": {
                      "kind": "List",
                      "list": [
                      ]
                    },
                    "visibility": "Public",
                    "arguments": [
                    ]
                  }
                ],
                "definition": "2676705278976 test1"
              },
              "connections": [
              ]
            }
          ],
          "definition": "2676705266688 test"
        },
        "connections": [
        ]
      }
    ]
  },
  "definitions": [
    {
      "name": "test",
      "kind": "Definition",
      "addr": 2676705266688,
      "defaultNetType": "2676705603936 wire",
      "definitionKind": "Module",
      "defaultLifetime": "Static",
      "unconnectedDrive": "None"
    },
    {
      "name": "test1",
      "kind": "Definition",
      "addr": 2676705278976,
      "defaultNetType": "2676705603936 wire",
      "definitionKind": "Module",
      "defaultLifetime": "Static",
      "unconnectedDrive": "None"
    }
  ]
}

But if we add a simple port into module test1 declaration:

module test();
    module test1(input clk);
        function void foo();
        endfunction
    endmodule
endmodule

slang generates an AST without any instance of test1 nor foo declaration:

{
  "design": {
    "name": "$root",
    "kind": "Root",
    "addr": 3589956581920,
    "members": [
      {
        "name": "",
        "kind": "CompilationUnit",
        "addr": 3589955966176
      },
      {
        "name": "test",
        "kind": "Instance",
        "addr": 3589955966800,
        "body": {
          "name": "test",
          "kind": "InstanceBody",
          "addr": 3589955966472,
          "definition": "3589956242432 test"
        },
        "connections": [
        ]
      }
    ]
  },
  "definitions": [
    {
      "name": "test",
      "kind": "Definition",
      "addr": 3589956242432,
      "defaultNetType": "3589956579680 wire",
      "definitionKind": "Module",
      "defaultLifetime": "Static",
      "unconnectedDrive": "None"
    },
    {
      "name": "test1",
      "kind": "Definition",
      "addr": 3589956254720,
      "defaultNetType": "3589956579680 wire",
      "definitionKind": "Module",
      "defaultLifetime": "Static",
      "unconnectedDrive": "None"
    }
  ]
}

Is this behaviour is correct?

MikePopoloski commented 1 month ago

This is correct. From 23.4:

Nested modules with no ports that are not explicitly instantiated shall be implicitly instantiated once with an
instance name identical to the module name. Otherwise, if they have ports and are not explicitly instantiated,
they are ignored.