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 'override' in virtual methods of derived classes. #4790

Merged
merged 1 commit into from
Mar 9, 2025

Conversation

hzeller
Copy link
Contributor

@hzeller hzeller commented Mar 8, 2025

... and clean up duplicates where a method is marked virtual and override (override implies `virtual).

Use of override informs the developer reading the code and reduces mistakes (e.g. accidentally messing up a signature while overridding).

... and clean up duplicates where a method is marked
`virtual` _and_ `override` (`override` implies `virtual).

Use of `override` informs the developer reading the code and reduces
mistakes (e.g. accidentally messing up a signature while
overridding).

Signed-off-by: Henner Zeller <[email protected]>
@hzeller hzeller force-pushed the feature-20250308-use-override branch from 4337ea6 to ef07d6a Compare March 8, 2025 19:39
@hzeller
Copy link
Contributor Author

hzeller commented Mar 8, 2025

FYI @parrt

@parrt
Copy link
Member

parrt commented Mar 9, 2025

hiya. gosh, that's a lot of changes, adding virtual everywhere etc! Is it worth the risk of such a big change? I will defer to your experience.

@hzeller
Copy link
Contributor Author

hzeller commented Mar 9, 2025

there is no risk. Also, if you noticced, most of the change is just removing the superfluous virtual in front where there is already override at the end.

@hzeller
Copy link
Contributor Author

hzeller commented Mar 9, 2025

or to put it this way: the code is identical to the previous one for all intents and purposes, just more readable.
The changes are made fully automatic to avoid errors.

@parrt
Copy link
Member

parrt commented Mar 9, 2025

Thanks :)

@parrt parrt merged commit ea9cf6a into antlr:dev Mar 9, 2025
42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants