Shut up warnings clang with libstdc++ emits about non-virtual destructors #1012
Conversation
DanielGibson
force-pushed
the
fix-clang-warnings
branch
from
0f10f89 to
faf9cae
Compare
|
Please rebase against latest main and add an entry to the matrix in |
DanielGibson
force-pushed
the
fix-clang-warnings
branch
from
faf9cae to
0006dcf
Compare
|
rebasing is done, I'll look into adding clang |
|
Added Clang support, I hope it works (we'll see after the workflow run has been approved) |
|
Please add a clang Release build too. You can just change config in the matrix entry to |
DanielGibson
force-pushed
the
fix-clang-warnings
branch
2 times, most recently
from
b004e03 to
7066356
Compare
|
ok, done, also rebased against current main |
.github/workflows/linux.yml
Outdated
| compiler: clang | ||
| vk_sdk_ver: '1.3.290' | ||
| options: { | ||
| config: Debug, Release, |
There was a problem hiding this comment.
Must be Debug,Release. No spaces and no trailing comma.
There was a problem hiding this comment.
Please fix. I want to get this merged.
There was a problem hiding this comment.
Unfortunately it is still not working. The env. var. CONFIGURATION is only being set to Debug. I think you need single quotes when setting the matrix value:
config: 'Debug,Release',
There was a problem hiding this comment.
doesn't work either
There was a problem hiding this comment.
I just duplicated the clang section now, once for Release and once for Debug
DanielGibson
force-pushed
the
fix-clang-warnings
branch
5 times, most recently
from
169c9e6 to
838c3f8
Compare
It complains a lot about non-final classes (structs) derived from KVEntry having virtual functions but no virtual destructor, which in std::optional code (that explicitly calls the destructor somewhere) could theoretically lead to the wrong destructor being called (that of the struct directly derived from KVEntry instead of the one of a hypothetical class derived from that struct that might actually need its destructor called). This doesn't matter in practice because 1. those classes don't have any implemented constructors that need to do anything 2. no further classes are derived from those that are derived from KVEntry So (I think!) these warnings are only hypothetical, but still annoying. Apparently this does not happen if Clang uses libc++ or MSVCs C++ stdlib (in the ClangCL case), but only with libstdc++, maybe even only with libstdc++ from GCC14, but that's the only version I have on my machine. fixes KhronosGroup#1011
DanielGibson
force-pushed
the
fix-clang-warnings
branch
from
838c3f8 to
1b81738
Compare
|
Sorry about all the troubles. This workflow needs to use the "Ninja Multi-Config" generator for the multi-config builds to work. The separate matrix entries you have just added are good for now. I'll work on converting to "Ninja Multi-Config" later. |
|
Thank you for your hard work on this. |
It complains a lot about non-final classes (structs) derived from KVEntry having virtual functions but no virtual destructor, which in std::optional code (that explicitly calls the destructor somewhere) could theoretically lead to the wrong destructor being called (that of the struct directly derived from KVEntry instead of the one of a hypothetical class derived from that struct that might actually need its destructor called).
This doesn't matter in practice because
So (I think!) these warnings are only hypothetical, but still annoying.
Apparently this does not happen if Clang uses libc++ or MSVCs C++ stdlib (in the ClangCL case), but only with libstdc++, maybe even only with libstdc++ from GCC14, but that's the only version I have on my machine.
Fixes #1011