Added NatSpec support for enum value definitions in the AST

[veniger]

Adds NatSpec field to AST node for enum value definitions Partially fixes: https://github.com/ethereum/solidity/issues/12295

[github-actions[bot]]

Thank you for your contribution to the Solidity compiler! A team member will follow up shortly.

If you haven't read our contributing guidelines and our review checklist before, please do it now, this makes the reviewing process and accepting your contribution smoother.

If you have any questions or need our help, feel free to post them in the PR or talk to us directly on the #solidity-dev channel on Matrix.

[r0qs]

@veniger just a note about the failing external tests (i.e. all the ones with *_test_ext_*). Please ignore them for now, since I just noticed that due to the changes in https://github.com/ethereum/solidity/pull/14242 the tests are failing for external contributions due to lack of access to the github token.

The errors in the soltests still need to be fixed in this PR though: https://app.circleci.com/pipelines/github/ethereum/solidity/29881/workflows/47cd436e-67ed-46b8-abe4-102c5f331b1d/jobs/1327332

[ekpyron]

@nikola-matic @veniger What's the state of this?

[nikola-matic]

@nikola-matic @veniger What's the state of this?

I can take a another more detailed look, but should be good enough to merge after a rebase.

[zerosnacks]

Hi all, would be great to get this reviewed and merged, cc @cameel / @nikola-matic

If a rebase is required, @veniger would you be able to do so?

Users are proposing to implement workarounds as seen here: https://github.com/foundry-rs/foundry/pull/9905 but I think it is much better to resolve the issue here.

[nikola-matic]

Hi all, would be great to get this reviewed and merged, cc @cameel / @nikola-matic

If a rebase is require, @veniger would you be able to do so?

Users are proposing to implement workarounds as seen here: foundry-rs/foundry#9905 but I think it is much better to resolve the issue here.

Sure, we have a release coming up soon, so I'll check with the team on Monday whether we have enough time to get this in for the upcoming release; if not, we'll get it in for the next one.

[zerosnacks]

Hi @nikola-matic, any updates on this? Would be great to get this in - thanks!

[nikola-matic]

Hi @nikola-matic, any updates on this? Would be great to get this in - thanks!

I discussed it with the team and we've decided for this to go in, however, not in this release. We're currently almost exclusively focused on merging all of the storage layout (account abstraction for EOAs) PRs and will have no time for this one, especially since we're planning on releasing this week. I marked this issue (and PR) for the next release.

edit: you won't have to wait long for the (next) 8.30 release since we have some other stuff we're working for tooling as well, it's just that our priorities at the moment are on compatibility for the next hard fork.

[zerosnacks]

Hi @nikola-matic, any updates on this? Would be great to get this in - thanks!

I discussed it with the team and we've decided for this to go in, however, not in this release. We're currently almost exclusively focused on merging all of the storage layout (account abstraction for EOAs) PRs and will have no time for this one, especially since we're planning on releasing this week. I marked this issue (and PR) for the next release.

edit: you won't have to wait long for the (next) 8.30 release since we have some other stuff we're working for tooling as well, it's just that our priorities at the moment are on compatibility for the next hard fork.

Understood, not a problem - looking forward to the next releases :+1:

[nikola-matic]

Hi @nikola-matic, any updates on this? Would be great to get this in - thanks!

I discussed it with the team and we've decided for this to go in, however, not in this release. We're currently almost exclusively focused on merging all of the storage layout (account abstraction for EOAs) PRs and will have no time for this one, especially since we're planning on releasing this week. I marked this issue (and PR) for the next release. edit: you won't have to wait long for the (next) 8.30 release since we have some other stuff we're working for tooling as well, it's just that our priorities at the moment are on compatibility for the next hard fork.

Understood, not a problem - looking forward to the next releases 👍

I closed this PR as it was old, and opened a new one, which was just merged. The enum support for Natspec will be available in the upcoming release.