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

Add a virtual destructor to the AbiLayout base class. #18907

Merged
merged 2 commits into from
Oct 14, 2016
Merged

Conversation

maleadt
Copy link
Member

@maleadt maleadt commented Oct 13, 2016

Since we're using ABI layout objects polymorphically (via std::unique_ptr<AbiLayout> abi), the base class needs a virtual destructor or the derived class' destructor will never trigger (it might even be UB).

This also squelches compile-time warnings:

In file included from src/codegen.cpp:1870:
In file included from src/intrinsics.cpp:7:
src/ccall.cpp:337:7: warning: 'AbiLayout' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor]
class AbiLayout {
      ^
In file included from src/codegen.cpp:1870:
In file included from src/intrinsics.cpp:7:
In file included from src/ccall.cpp:363:
src/abi_llvm.cpp:41:8: warning: 'ABI_LLVMLayout' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor]
struct ABI_LLVMLayout : AbiLayout {
       ^
In file included from src/codegen.cpp:1870:
In file included from src/intrinsics.cpp:7:
In file included from src/ccall.cpp:365:
src/abi_arm.cpp:24:8: warning: 'ABI_ARMLayout' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor]
struct ABI_ARMLayout : AbiLayout {
       ^
In file included from src/codegen.cpp:1870:
In file included from src/intrinsics.cpp:7:
In file included from src/ccall.cpp:366:
src/abi_aarch64.cpp:14:8: warning: 'ABI_AArch64Layout' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor]
struct ABI_AArch64Layout : AbiLayout {
       ^
In file included from src/codegen.cpp:1870:
In file included from src/intrinsics.cpp:7:
In file included from src/ccall.cpp:367:
src/abi_ppc64le.cpp:42:8: warning: 'ABI_PPC64leLayout' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor]
struct ABI_PPC64leLayout : AbiLayout {
       ^
In file included from src/codegen.cpp:1870:
In file included from src/intrinsics.cpp:7:
In file included from src/ccall.cpp:368:
src/abi_win32.cpp:40:8: warning: 'ABI_Win32Layout' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor]
struct ABI_Win32Layout : AbiLayout {
       ^
In file included from src/codegen.cpp:1870:
In file included from src/intrinsics.cpp:7:
In file included from src/ccall.cpp:369:
src/abi_win64.cpp:46:8: warning: 'ABI_Win64Layout' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor]
struct ABI_Win64Layout : AbiLayout {
       ^
In file included from src/codegen.cpp:1870:
In file included from src/intrinsics.cpp:7:
In file included from src/ccall.cpp:370:
src/abi_x86_64.cpp:41:8: warning: 'ABI_x86_64Layout' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor]
struct ABI_x86_64Layout : AbiLayout {
       ^
In file included from src/codegen.cpp:1870:
In file included from src/intrinsics.cpp:7:
In file included from src/ccall.cpp:371:
src/abi_x86.cpp:40:8: warning: 'ABI_x86Layout' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor]
struct ABI_x86Layout : AbiLayout {
       ^

@maleadt
Copy link
Member Author

maleadt commented Oct 13, 2016

Fixed a similar warning in the second commit:

In file included from src/codegen.cpp:1870:
In file included from src/intrinsics.cpp:7:
src/ccall.cpp:769:7: warning: 'FunctionMover' has virtual functions but non-virtual destructor [-Wnon-virtual-dtor]
class FunctionMover : public ValueMaterializer

FunctionMover is meant to be used non-polymorphically (ValueMaterializer's ctors are protected), so the base dtor isn't virtual. Making FunctionMover final and removing its virtual methods makes that we don't need a virtual dtor ourselves (only removing the virtual methods isn't sufficient because we inherit other virtual methods which we don't override).

@maleadt maleadt merged commit b5fe42d into master Oct 14, 2016
@tkelman tkelman deleted the tb/virtual_dtor branch October 14, 2016 20:08
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.

1 participant