Skip to content

Conversation

@SolidWallOfCode
Copy link
Member

This will prevent potential future issues by following the rule of any class that needs virtual methods must have a virtual destructor.

@SolidWallOfCode SolidWallOfCode added this to the 10.0.0 milestone Jan 27, 2020
@SolidWallOfCode SolidWallOfCode self-assigned this Jan 27, 2020
Copy link
Contributor

@ywkaras ywkaras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nobrainer.

@SolidWallOfCode SolidWallOfCode merged commit 11aeedf into apache:master Jan 27, 2020
@masaori335
Copy link
Contributor

Obviously, this is the way.

@ywkaras
Copy link
Contributor

ywkaras commented Jan 29, 2020

Assuming optimal vague linkage, there will only be one copy of the virtual table for each dynamic class in memory. An additional virtual function only adds a pointer and a this pointer offset to the virtual table. So a small price to cover the case when the virtual destructor is actually needed (instance of a derived class is created in the heap and deleted using a pointer to its base class). https://stackoverflow.com/a/461224

@zwoop
Copy link
Contributor

zwoop commented Feb 21, 2020

I believe this depends on 48bcbe6

@zwoop zwoop added the OnDocs This is for PR currently running, or will run, on the Docs ATS server label Feb 21, 2020
@zwoop
Copy link
Contributor

zwoop commented Feb 27, 2020

Cherry-picked to v9.0.x branch.

@zwoop zwoop removed the OnDocs This is for PR currently running, or will run, on the Docs ATS server label Feb 27, 2020
@zwoop zwoop modified the milestones: 10.0.0, 9.0.0 Mar 9, 2020
@zwoop zwoop modified the milestones: 9.0.0, 8.1.0 Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants