Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use VAST's fallthrough visitor #33

Closed
PappasBrent opened this issue Jun 17, 2024 · 0 comments · Fixed by #44
Closed

Use VAST's fallthrough visitor #33

PappasBrent opened this issue Jun 17, 2024 · 0 comments · Fixed by #44
Assignees
Labels
dependencies External project dependencies enhancement ✨ New feature or request priority:low Low-priority tasks

Comments

@PappasBrent
Copy link
Collaborator

PappasBrent commented Jun 17, 2024

Macroni's visitors should inherit from VAST's new fallthrough_visitor struct so that they only need to define the visit methods relevant to them.

Currently, each of Macroni's visitors that inherit from visitor_base need to define all their visit method in order to be concrete structs. However, this adds a lot of boilerplate code since Macroni's visitors only do special actions for one or two visit methods, and just return nullptr for all else. So to reduce boilerplate Macroni's visitors should inherit from a struct that already defines all its visitors to return nullptr. There are PRs open which use a custom-defined empty visitor; however once VAST merges this commit Macroni's structs should just inherit from the fallthrough_visitor.

@PappasBrent PappasBrent added enhancement ✨ New feature or request priority:low Low-priority tasks dependencies External project dependencies labels Jun 17, 2024
@PappasBrent PappasBrent self-assigned this Jun 17, 2024
@PappasBrent PappasBrent changed the title Use updated vast's fallthrough visitor Use VAST's fallthrough visitor Jun 17, 2024
@PappasBrent PappasBrent mentioned this issue Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies External project dependencies enhancement ✨ New feature or request priority:low Low-priority tasks
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant