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

Update File to mark overridden functions as 'override' #772

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ssilverman
Copy link
Contributor

@ssilverman ssilverman commented Nov 29, 2024

No description provided.

@ssilverman ssilverman force-pushed the fix-fs-mark-functions-override branch 3 times, most recently from a7bff4f to 801cec2 Compare November 29, 2024 22:38
@ssilverman
Copy link
Contributor Author

ssilverman commented Nov 29, 2024

Note to reviewer: I can use 'virtual' instead, if you like, but the following PR will change all overridden methods to use 'override' instead of 'virtual': #771.

@A-Dunstan
Copy link
Contributor

Without 'virtual' or 'override', the default Stream and Print functions will be used instead.

This shouldn't be the case. If a derived class implements a virtual method from the base class it automatically replaces it without any extra notation required. Specifying "override" is just a precaution that will make the compiler throw an error if there isn't a matching virtual base method.

@ssilverman
Copy link
Contributor Author

ssilverman commented Dec 28, 2024

Without 'virtual' or 'override', the default Stream and Print functions will be used instead.

This shouldn't be the case. If a derived class implements a virtual method from the base class it automatically replaces it without any extra notation required. Specifying "override" is just a precaution that will make the compiler throw an error if there isn't a matching virtual base method.

Oops, you're right. I'll update the PR description and title. Overridden/virtual/etc functions should still be marked as such. It's good practice so that people reading the code don't have to guess or research every function.

Relevant portions of the spec:
https://en.cppreference.com/w/cpp/language/virtual:

Then this function in the class Derived is also virtual (whether or not the keyword virtual is used in its declaration) and overrides Base::vf (whether or not the word override is used in its declaration).

@ssilverman ssilverman changed the title Fix File to mark overridden functions as 'override' Update File to mark overridden functions as 'override' Dec 28, 2024
@ssilverman ssilverman force-pushed the fix-fs-mark-functions-override branch from 801cec2 to 53a7c6e Compare December 28, 2024 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants