chipsalliance / verible

Verible is a suite of SystemVerilog developer tools, including a parser, style-linter, formatter and language server
https://chipsalliance.github.io/verible/
Other
1.38k stars 214 forks source link

wrong CST of `for (uint x = ....)`, "uint" should be wrapped in a data type #548

Closed MinaToma closed 4 years ago

MinaToma commented 4 years ago

Describe the bug

uint is parsed as kReferenceCallBase and not as kDataTypePrimitive in kForInitialization.

To Reproduce

module m();
   initial begin
      for (uint x = 0; x < 5; x++) begin
      end
   end
endmodule

Actual behavior:

Current CST:

Node @0 (tag: kForInitialization) {
                        Node @1 (tag: kReferenceCallBase) {
                          Node @0 (tag: kReference) {
                            Node @0 (tag: kLocalRoot) {
                              Node @0 (tag: kUnqualifiedId) {
                                Leaf @0 (#SymbolIdentifier @42-46: "uint")
                              }
                            }
                          }
                        }
                        Leaf @2 (#SymbolIdentifier @47-48: "x")
                        Leaf @3 (#'=' @49-50: "=")
                        Node @4 (tag: kExpression) {
                          Node @0 (tag: kNumber) {
                            Leaf @0 (#TK_DecNumber @51-52: "0")
                          }
                        }
                      }
                    }

Expected behavior

Expected CST:

Node @0 (tag: kForInitialization) {
                        Node @1 (tag: kDataTypePrimitive) {
                          Leaf @0 (#"int" @42-45: "int")
                          Node @2 (tag: kPackedDimensions) {
                          }
                        }
                        Leaf @2 (#SymbolIdentifier @46-47: "x")
                        Leaf @3 (#'=' @48-49: "=")
                        Node @4 (tag: kExpression) {
                          Node @0 (tag: kNumber) {
                            Leaf @0 (#TK_DecNumber @50-51: "0")
                          }
                        }
                      }
                    }
MinaToma commented 4 years ago

@fangism can you add this issue to the right project.

fangism commented 4 years ago

It doesn't have to be a kDataTypePrimitive, but it should at least be wrapped in kDataType like any other declaration.

The parsing is correct, just the CST subtree is not re-interpreted properly.

fangism commented 4 years ago

Today we get:

            Node @0 (tag: kForLoopStatement) {
              Node @0 (tag: kLoopHeader) {
                Leaf @0 (#"for" @39-42: "for")
                Node @1 (tag: kParenGroup) {
                  Leaf @0 (#'(' @43-44: "(")
                  Node @1 (tag: kForSpec) {
                    Node @0 (tag: kForInitializationList) {
                      Node @0 (tag: kForInitialization) {
                        Node @1 (tag: kDataType) {
                          Node @1 (tag: kUnqualifiedId) {
                            Leaf @0 (#SymbolIdentifier @44-48: "uint")
                          }
                          Node @3 (tag: kPackedDimensions) {
                          }
                        }
                        Leaf @2 (#SymbolIdentifier @49-50: "x")
                        Leaf @3 (#'=' @51-52: "=")
                        Node @4 (tag: kExpression) {
                          Node @0 (tag: kNumber) {
                            Leaf @0 (#TK_DecNumber @53-54: "0")
                          }
                        }
                      }
                    }
                    Leaf @1 (#';' @54-55: ";")

So I believe this was fixed by https://github.com/google/verible/commit/259321c5bb95882d2ac872ca03de6c8d35fca733

fangism commented 4 years ago

I'll update a test to cover this.