MikePopoloski / slang

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

Added printing of source line info #1011

Closed KennethDRoe closed 2 weeks ago

KennethDRoe commented 1 month ago

I think that having source file/line info in the json outputs will be useful to the community as a whole. I'm submitting a pull request to contribute this change.

MikePopoloski commented 4 weeks ago

Thanks for the PR. In case it wasn't clear, I can't land this until the tests are fixed.

Also I think this output should be hidden behind a command line flag. The JSON output is already very large for real world designs, and this will make it much larger. I suspect many people won't want to see this in the output, so it should be controlled via the flag.

KennethDRoe commented 4 weeks ago

I’ll add a flag and fix the tests. It make a few days.             - KenSent from my iPhoneOn May 29, 2024, at 8:43 PM, Michael Popoloski @.***> wrote: Thanks for the PR. In case it wasn't clear, I can't land this until the tests are fixed. Also I think this output should be hidden behind a command line flag. The JSON output is already very large for real world designs, and this will make it much larger. I suspect many people won't want to see this in the output, so it should be controlled via the flag.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

codecov[bot] commented 3 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.70%. Comparing base (a4d08eb) to head (27e3657).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/MikePopoloski/slang/pull/1011/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/1011?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 #1011 +/- ## ======================================= Coverage 94.69% 94.70% ======================================= Files 191 191 Lines 47553 47569 +16 ======================================= + Hits 45031 45048 +17 + Misses 2522 2521 -1 ``` | [Files](https://app.codecov.io/gh/MikePopoloski/slang/pull/1011?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski) | Coverage Δ | | |---|---|---| | [include/slang/ast/ASTSerializer.h](https://app.codecov.io/gh/MikePopoloski/slang/pull/1011?src=pr&el=tree&filepath=include%2Fslang%2Fast%2FASTSerializer.h&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski#diff-aW5jbHVkZS9zbGFuZy9hc3QvQVNUU2VyaWFsaXplci5o) | `100.00% <100.00%> (ø)` | | | [source/ast/ASTSerializer.cpp](https://app.codecov.io/gh/MikePopoloski/slang/pull/1011?src=pr&el=tree&filepath=source%2Fast%2FASTSerializer.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Michael+Popoloski#diff-c291cmNlL2FzdC9BU1RTZXJpYWxpemVyLmNwcA==) | `91.19% <100.00%> (+0.91%)` | :arrow_up: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/MikePopoloski/slang/pull/1011/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/1011?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/1011?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 [a4d08eb...27e3657](https://app.codecov.io/gh/MikePopoloski/slang/pull/1011?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 2 weeks ago

It seems that the tests are still failing in the CI build. Also I got access to a Macbook M3 and the tests all pass locally there, so I'm not sure why you're unable to get them to pass on your machine unless the problem is specific to the M1s.

KennethDRoe commented 2 weeks ago

The failures on my machine may be due to the software stack (and not the M1 processor).

Can you send over the list of failures you are seeing with my pull request? Also, make sure you have the latest version of files from my fork. I made a couple of last minute fixes.

              - Ken

On Sun, Jun 9, 2024 at 10:53 PM Michael Popoloski @.***> wrote:

It seems that the tests are still failing in the CI build. Also I got access to a Macbook M3 and the tests all pass locally there, so I'm not sure why you're unable to get them to pass on your machine unless the problem is specific to the M1s.

— Reply to this email directly, view it on GitHub https://github.com/MikePopoloski/slang/pull/1011#issuecomment-2156663693, or unsubscribe https://github.com/notifications/unsubscribe-auth/BIIJFIPNXN4RIZTXJ6HGQ3LZGR26RAVCNFSM6AAAAABIJVMH3GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJWGY3DGNRZGM . You are receiving this because you authored the thread.Message ID: @.***>

MikePopoloski commented 2 weeks ago

The CI results are on this thread of the PR. For example: https://github.com/MikePopoloski/slang/actions/runs/9364528795/job/25888520547?pr=1011

KennethDRoe commented 2 weeks ago

I believe I fixed the one failing test. Try again.

        - Ken

On Tue, Jun 11, 2024 at 5:38 AM Michael Popoloski @.***> wrote:

The CI results are on this thread of the PR. For example: https://github.com/MikePopoloski/slang/actions/runs/9364528795/job/25888520547?pr=1011

— Reply to this email directly, view it on GitHub https://github.com/MikePopoloski/slang/pull/1011#issuecomment-2159425985, or unsubscribe https://github.com/notifications/unsubscribe-auth/BIIJFIOWRS5SPGMKG6GNMZTZGYTEVAVCNFSM6AAAAABIJVMH3GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJZGQZDKOJYGU . You are receiving this because you authored the thread.Message ID: @.***>