MikePopoloski / slang

SystemVerilog compiler and language services
MIT License
558 stars 122 forks source link

[slang][lang-compliance] Introduce nondegeneracy check #998

Closed likeamahoney closed 1 month ago

likeamahoney commented 1 month ago

Based on 16.12.22 SystemVerilog LRM section and partially (F.4.3/F.5.2/F.5.5).

Also Ben Cohen's paper about nondegeneracy understanding from here.

Complex sequences are not often used in industrial/open source designs, but I think these checks will help to verify restrictions from 16.12.22 in most cases.

I written about ~300 lines of sv tests but i need help with more complex test cases but if anyone could also come up with more complex tests to test heuristics, that would be nice.

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 74.46043% with 71 lines in your changes are missing coverage. Please review.

Project coverage is 94.14%. Comparing base (b1354ad) to head (a4b2714).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/MikePopoloski/slang/pull/998/graphs/tree.svg?width=650&height=150&src=pr&token=sS5JjK9091&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski)](https://app.codecov.io/gh/MikePopoloski/slang/pull/998?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski) ```diff @@ Coverage Diff @@ ## master #998 +/- ## ========================================== - Coverage 94.24% 94.14% -0.10% ========================================== Files 191 191 Lines 47344 47568 +224 ========================================== + Hits 44618 44785 +167 - Misses 2726 2783 +57 ``` | [Files](https://app.codecov.io/gh/MikePopoloski/slang/pull/998?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski) | Coverage Δ | | |---|---|---| | [source/ast/expressions/MiscExpressions.cpp](https://app.codecov.io/gh/MikePopoloski/slang/pull/998?src=pr&el=tree&filepath=source%2Fast%2Fexpressions%2FMiscExpressions.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski#diff-c291cmNlL2FzdC9leHByZXNzaW9ucy9NaXNjRXhwcmVzc2lvbnMuY3Bw) | `94.11% <100.00%> (+<0.01%)` | :arrow_up: | | [include/slang/ast/expressions/AssertionExpr.h](https://app.codecov.io/gh/MikePopoloski/slang/pull/998?src=pr&el=tree&filepath=include%2Fslang%2Fast%2Fexpressions%2FAssertionExpr.h&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski#diff-aW5jbHVkZS9zbGFuZy9hc3QvZXhwcmVzc2lvbnMvQXNzZXJ0aW9uRXhwci5o) | `55.65% <22.58%> (-7.79%)` | :arrow_down: | | [source/ast/expressions/AssertionExpr.cpp](https://app.codecov.io/gh/MikePopoloski/slang/pull/998?src=pr&el=tree&filepath=source%2Fast%2Fexpressions%2FAssertionExpr.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski#diff-c291cmNlL2FzdC9leHByZXNzaW9ucy9Bc3NlcnRpb25FeHByLmNwcA==) | `87.45% <80.81%> (-2.75%)` | :arrow_down: | ... and [2 files with indirect coverage changes](https://app.codecov.io/gh/MikePopoloski/slang/pull/998/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski) ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/MikePopoloski/slang/pull/998?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/MikePopoloski/slang/pull/998?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski). Last update [b1354ad...a4b2714](https://app.codecov.io/gh/MikePopoloski/slang/pull/998?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski).
MikePopoloski commented 1 month ago

Thank you for tackling this; the sections of the LRM that address this are daunting which is why I had not implemented it yet. I'll have to review this quite closely.

One thing that can help inform additional tests is looking at the coverage report, which shows some big chunks that haven't been tested at all yet.

likeamahoney commented 1 month ago

Thank you for tackling this; the sections of the LRM that address this are daunting which is why I had not implemented it yet. I'll have to review this quite closely.

One thing that can help inform additional tests is looking at the coverage report, which shows some big chunks that haven't been tested at all yet.

At the moment I only added a few tests to increase the possible coverage but at all it still in progress

likeamahoney commented 1 month ago

Thank you for tackling this; the sections of the LRM that address this are daunting which is why I had not implemented it yet. I'll have to review this quite closely. One thing that can help inform additional tests is looking at the coverage report, which shows some big chunks that haven't been tested at all yet.

At the moment I only added a few tests to increase the possible coverage but at all it still in progress

And also add support for checking nondegenerancy for lhs of "followed-by property" properties (e.g. #-# and #=#)

MikePopoloski commented 1 month ago

Ok, I think this is good enough to land -- we can tweak or improve it later if needed.