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

Potential lack of keyword final #23

Open
Xenoamor opened this issue Jun 30, 2019 · 6 comments
Open

Potential lack of keyword final #23

Xenoamor opened this issue Jun 30, 2019 · 6 comments

Comments

@Xenoamor
Copy link

Xenoamor commented Jun 30, 2019

void write( bool x ) override {

Looking at this line should the keyword final not be used? If this is omitted then a vtable would need to be created and the generated code would bloat.

I've made an example of this on godbolt

@wovo
Copy link
Owner

wovo commented Jun 30, 2019 via email

@Xenoamor
Copy link
Author

Xenoamor commented Jun 30, 2019

Hi Wouter, I think you might have misunderstood me. It is correct to have the keyword override but it would be beneficial to also have the keyword final.
If you do then the compiler can de-virtualise it and remove the overheads associated with it. You'll see this optimisation occurring in the godbolt link above also

I'd also like to add that I saw your talk on youtube from a recommendation on Reddit and would like to thank you for it! It's been a great help in selling me on moving to C++

@wovo
Copy link
Owner

wovo commented Jun 30, 2019 via email

@Xenoamor
Copy link
Author

Awesome, it'll be interesting to know if it becomes as fast as a templated version. I imagine link time optimisation might be needed for that though

@wovo
Copy link
Owner

wovo commented Jun 30, 2019 via email

@Xenoamor
Copy link
Author

Yes I see this now, I've played around with it in godbolt and you only get the savings from final if you are directly calling the final class. If you call anything above you get the intermediate objects as you say.

I'll have a look at godafoss when I get some free time!

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

No branches or pull requests

2 participants