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.39k stars 215 forks source link

[kythe] `branch` lexed as a (AMS) keyword, not identifier #547

Open MinaToma opened 4 years ago

MinaToma commented 4 years ago

Input test case

class riscv_jump_instr extends riscv_directed_instr_stream;
    riscv_instr          branch;
endclass

Describe what is wrong or missing

CST for this code:

Node @3 (tag: kDataDeclaration) {
        Node @1 (tag: kInstantiationBase) {
          Node @0 (tag: kInstantiationType) {
            Node @0 (tag: kReferenceCallBase) {
              Node @0 (tag: kReference) {
                Node @0 (tag: kLocalRoot) {
                  Node @0 (tag: kUnqualifiedId) {
                    Leaf @0 (#SymbolIdentifier @3323-3334: "riscv_instr")
                  }
                }
              }
            }
          }
          Node @1 (tag: kVariableDeclarationAssignmentList) {
            Node @0 (tag: kVariableDeclarationAssignment) {
              Leaf @0 (#"branch" @3344-3350: "branch")
              Node @1 (tag: kUnpackedDimensions) {
              }
            }
          }
        }
        Leaf @2 (#';' @3350-3351: ";")
      }

Expected CST:

Node @3 (tag: kDataDeclaration) {
        Node @1 (tag: kInstantiationBase) {
          Node @0 (tag: kInstantiationType) {
            Node @0 (tag: kReferenceCallBase) {
              Node @0 (tag: kReference) {
                Node @0 (tag: kLocalRoot) {
                  Node @0 (tag: kUnqualifiedId) {
                    Leaf @0 (#SymbolIdentifier @3323-3334: "riscv_instr")
                  }
                }
              }
            }
          }
          Node @1 (tag: kVariableDeclarationAssignmentList) {
            Node @0 (tag: kVariableDeclarationAssignment) {
              Leaf @0 (#SymbolIdentifier @3344-3350: "branch")
              Node @1 (tag: kUnpackedDimensions) {
              }
            }
          }
        }
        Leaf @2 (#';' @3350-3351: ";")
      }

In CST branch here should be SymbolIdentifier and not branch

fangism commented 4 years ago

Not a kythe issue so much as it is a lexer issue:

https://cs.opensource.google/verible/verible/+/master:verilog/parser/verilog.y;drc=cb7d6286758ebbb446cb9ccb87d5bfff350e94cf;l=533 https://cs.opensource.google/verible/verible/+/master:verilog/parser/verilog.y;drc=cb7d6286758ebbb446cb9ccb87d5bfff350e94cf;l=793

branch is a Verilog-AMS keyword, and conservatively, the lexer and parser interpret it as such. I don't have dialect selection implemented yet.

fangism commented 4 years ago

Dialect-selection would mean a configurable post-lexer pass that downgrades select keywords into SymbolIdentifier before passing it onto the parser.

fangism commented 4 years ago

Status: Currently, this is worked around in Verible's kythe extractor, the branch keyword (when used as an identifier) is ignored safely.

hzeller commented 2 years ago

Is fixed since https://github.com/chipsalliance/verible/commit/ef935c9036984a4583df5510f813f4e66db0799f in which branch is qualified as SymbolIdentifier as it is never used in the grammar.