Adds highlighting support for multi-line declaration statements

[vinamarora8]

Example:

[vinamarora8]

The commit ae9c1c7 isn't exactly related to the title. But since it's a very small change, I decided to club it into the same pull request.

[wsnyder]

Can you please benchmark?

Find/make (by concatting) a very large file that takes several seconds to font-lock and time with and without this. You might need to turn up your font-lock-maximum-size.

[acr4]

An easy way to do this is to grab a gate-level Verilog file. Those can be hundreds of megabytes and font-lock cripples my wimpy corporate virtual machine.

On Mon, Jun 15, 2020 at 9:36 AM Wilson Snyder notifications@github.com wrote:

Can you please benchmark?

Find/make (by concatting) a very large file that takes several seconds to font-lock and time with and without this. You might need to turn up your font-lock-maximum-size.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/veripool/verilog-mode/pull/1681#issuecomment-644139642, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMKS2JZDMH7AJ5U32QZOTTRWYPXDANCNFSM4N5TRI5Q .

[vinamarora8]

I've found a lot of ways to 'fool' this highlighting scheme. So I'll spend some time fixing all those issues first, before benchmarking.

[vinamarora8]

I did some benchmarking. The file tested had 512 copies of all RTL in this repo. That totaled to about 88MB and 2.7M lines. Without any declaration highlighting, an average of 30 tries resulted in a fontification time of 0.295s. With 8a84d7a applied, fontification time was 0.310s, averaged over 30 tries. So that's a 5% increment.

[vinamarora8]

Now it seems to handle well everything I could throw at it.

[vinamarora8]

Bump

[wsnyder]

When I run "make && make test" in the distro I get a bunch of miscompares. Please update any test indentation necessary, and also add a test case for the multiline cases you are fixing.

More critically emacs seems to crash, so some additional debug is probably needed.

[vinamarora8]

Sorry for the long silence.

The tests fail due to line 2787. I added "localparam" and "parameter" to verilog-declaration-core-re, for the same reason that the list has "input" and "output". localparam and param are declaration prefixes but, just like input and output, can be used by themselves. Adding the line 2787 allows fontification of such parameter declarations. Since without it, parameter A = 10; won't be considered as a declaration statement. And hence, will not be fontified.

Since the list verilog-declaration-core-re is used for indentation as well (by verilog-pretty-declarations), indentation behavior is changed. Now parameter statements (with only parameter as the type) are also treated as declarations statements (which should be the default behavior imo). This is why we get mis-compares in the automated tests.

Maybe it'll be better to make a separate PR just for this change, if we decide to go for it. Since we'll have to update the test outputs too.

[vinamarora8]

74a94eb removes parameters from verilog-declaration-core-re which restores old indentation behavior. All tests pass now.

[wsnyder]

Thanks for the update. This looks like good solid work.

I'm open to changing parameter/localparam in another pull if you want to, or leaving as-is.

[wsnyder]

BTW there's unfortunately a largeish backlog of indentation bugs as that section of code has been looking for a maintainer, if you'd be willing to take any on it would be appreciated.

[vinamarora8]

Thanks!

I'm not at all familiar with the indentation code base, but I'm willing to give it a shot.