Skip to content
New issue

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

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

Already on GitHub? # to your account

Add default virtual destructor for Parser class. #2541

Conversation

zufuliu
Copy link
Contributor

@zufuliu zufuliu commented Dec 5, 2024

This fixed clang [-Wdelete-non-abstract-non-virtual-dtor] warning:
delete called on non-final 'ManifestParser' that has virtual
functions but non-virtual destructor.
for subparser_.reset(new ManifestParser(state_, file_reader_, options_));.

@digit-google
Copy link
Contributor

Thanks, I think it would make more sense to add a virtual (default) destructor to the Parser base class instead. Can you do that?

This fixed clang `[-Wdelete-non-abstract-non-virtual-dtor]` warning:
    delete called on non-final 'ManifestParser' that has virtual
    functions but non-virtual destructor.
for `subparser_.reset(new ManifestParser(state_, file_reader_, options_));`.
@zufuliu zufuliu force-pushed the fix-clang-Wdelete-non-abstract-non-virtual-dtor branch from 5be431f to 32e9958 Compare December 9, 2024 10:53
@zufuliu zufuliu changed the title Mark ManifestParser class as final. dd default virtual destructor for Parser class. Dec 9, 2024
@zufuliu zufuliu changed the title dd default virtual destructor for Parser class. Add default virtual destructor for Parser class. Dec 9, 2024
@digit-google
Copy link
Contributor

Thanks, this looks good to me. @jhasse , wdyt?

@jhasse jhasse merged commit 127f2d3 into ninja-build:master Dec 10, 2024
11 checks passed
@jhasse
Copy link
Collaborator

jhasse commented Dec 10, 2024

thx :)

@jhasse jhasse added this to the 1.13.0 milestone Jan 28, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants