vivekmalneedi / veridian

A SystemVerilog Language Server
MIT License
130 stars 15 forks source link

Add support for Verilator as a diagnostic backend in Veridian #176

Closed hilbert-yaa closed 6 months ago

hilbert-yaa commented 1 year ago

Please refer to my feature request at #175.

The following changes are made:

vivekmalneedi commented 1 year ago

Nice work, this looks really good! The patched version of lexical-core is an interesting idea as well. I attempted a full dep upgrade for #174 but the parser tests ended up being too brittle to get passing. A few requirements for this to be merged:

hilbert-yaa commented 1 year ago

Nice work, this looks really good! The patched version of lexical-core is an interesting idea as well. I attempted a full dep upgrade for #174 but the parser tests ended up being too brittle to get passing. A few requirements for this to be merged:

  • passing CI
  • there needs to be a test added for verilator (you'll need to install verilator in ci.yml)
  • the readme needs to be updated to document the new config options

Thanks for the kind response. Sounds good I'll work towards these points.

hilbert-yaa commented 1 year ago

I realized that adding a "-Wno-context" arg to Verilator would make the get_diagnostic pass faster. Shall we default this Verilator arg (i.e. "hardcode" it) inside Veridian?

I'll work on resolving test and CI issues and also add some tests & docs tmr.

vivekmalneedi commented 1 year ago

I realized that adding a "-Wno-context" arg to Verilator would make the get_diagnostic pass faster. Shall we default this Verilator arg (i.e. "hardcode" it) inside Veridian?

That's a good idea, I was wondering if it had an option to made the output more machine-readable

hilbert-yaa commented 1 year ago

I realized that adding a "-Wno-context" arg to Verilator would make the get_diagnostic pass faster. Shall we default this Verilator arg (i.e. "hardcode" it) inside Veridian?

That's a good idea, I was wondering if it had an option to made the output more machine-readable

Yes exactly, with "-Wno-context" Verilator will not output code blame info on each error (but may still contain some non-error, non-warning lines if my memory serves), shrinking the output size by 2-3x. Thus it will reduce the latency on filtering (tho filtering is still needed).

hakan-demirli commented 8 months ago

Any update on this? I would like to fix the blockers.

vivekmalneedi commented 8 months ago

@hakan-demirli sorry for the delayed response. I don't believe that there are any blockers apart from rebasing the changes onto master and testing which I've created a new PR to complete.

vivekmalneedi commented 6 months ago

PR merged in #188