Empty Range for Diagnostics

[SethGower]

Empty Range for Diagnostics

[SethGower]

I have made a bit of progress. Got the static checker working properly with the ranges. Will be working on seeing if I can get the builders to do it appropriately.

https://github.com/SethGower/hdl_checker/tree/range-fixes

[suoto]

Hi I think the best solution is to just send end column = start column + 1.

ModelSim, GHDL and Vivado have no message where they return both start and end columns and I could not think of any way to guess the end column reliably without slowing down the entire server.

I saw you're using the text in quotes to do that, but a quick test illustrates what I mean. This code

entity some_entity is
end some_entity_name;

Produces the message

Line 1, column 21: Labels do not match: 'some_entity' and 'some_entity_name'.

So if you take column 21 (it's in the end) and use the length of some_entity to infer where the range starts, you'd get column 10 and the editor would show

which is also not correct.

---

All things considered, the error text showing up next to the line that's causing it is good enough -- adding better column ranges is only marginally better.

[SethGower]

Ok, that makes sense. What builder were you using for that example? I tried using GHDL (don't have vivado installed, and msim doesn't provide column numbers). GHDL doesn't give the column of the end of the token, but the beginning.

test1.vhd:2:5:error: misspelling, "some_entity" expected
ghdl:error: compilation error

Output of ghdl --version

GHDL 1.0.0 (tarball) [Dunoon edition]
 Compiled with GNAT Version: 11.1.0
 llvm code generator
Written by Tristan Gingold.

Copyright (C) 2003 - 2021 Tristan Gingold.
GHDL is free software, covered by the GNU General Public License.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

[suoto]

I used modelsim for that example.

The error message parsing won't really care where the column is, the only difference is that using ghdl would result in an error in column 5 while modelsim would mark column 21, which I find acceptable to be honest.

You can think of a hierarchy of messages:

• (some messages are outright ignored)
• messages without line or column are placed in the first line and first character
• if there's a line number but no column, place the message in the first colmnn
• if there's a line and a column, place the message at that character (nothing is inferred)

That being said, there's some static checks (for example unused signals, ports, etc) which are parsed in Python and in that case there can be a real range rather than a single character.

(hopefully I'm not making things more confusing!)

[SethGower]

That makes sense. I feel like it can be done so that it won't cause errors with the other builders. Like if we know the behavior from modelsim (which column it says) it can be modified in the msim.py so that it doesn't affect anything else.

Now that you mention the static checks, I have that working and I believe that should be appropriate ranges. Currently only have a bug that it doesn't calculate them properly when more than one signal is declared on one line, like

signal a,b,c : std_logic;

I believe it's an issue with the regex, since it gives the range as being the start column where the name is, and then the length of the whole block of signal names + start as the end. So the diagnostic for that example would extend 5 characters after c. It's weird, but I am sure I can figure it out. I was on vacation last week, so I didn't do any work on it.

My big problem was the columns provided by the server break the LSP spec. So if you want to just do the first column as the location of an error, then the range should be 0:1.