Clang fails to export a single default destructor (out of thousands) #143
Comments
Somehow clang fails to properly export the default destructor. Should be revered when android/ndk#143 is fixed. Change-Id: I842f4141f249181ec871634b947e1c08470ddb83 Reviewed-by: Eskil Abrahamsen Blomfeldt <[email protected]>
|
@bog-dan-ro Can you provide instructions to reproduce the issue? |
|
Yep, you must compile the entire Qt in order to reproduce it. @pirama-arumuga-nainar If you need more info on this matter, please feel free to contact me (or other folks) on freenode #necessitas channel, or here. |
|
Is this with r12b? My build with r12b completed succesfully. I am doing a clean rebuild to double check. |
|
Can you double check if you've reverted my patch? |
|
I am able to see that error if I run make in qtconnectivity. Not sure why the top-level build succeeds. I am trying to drop-in a newer Clang and reproduce. |
|
It doesn't succeed, it's just hidden by the BTW I created two more patches for clang:
You need to apply these two patches to qtbase, and you don't need to rebuild everything! |
|
There is an ODR violation here.
OTOH, the definition of One possible design here seems to be for the class in |
|
PS: Not sure why gcc generates the destructor. Finding which object file among the gcc-built qllcpserver.o and qllcpserver_android.o has the destructor may shed some light. |
|
Thanks for tracking that down, @pirama-arumuga-nainar! |
|
Thanks a lot @pirama-arumuga-nainar, it was my fault because I didn't checked the code properly, sorry... |
Post-push fix with workaround for clang 6.0.0 and clang 7.0.0 linking errors such as:
```
/usr/bin/ld.lld: error: undefined symbol: locksys::Latches::~Latches()
>>> referenced by lock0lock.cc:322 (/home/mhansson/gitroot/worktrees/with_time_zone/mysql-trunk/storage/innobase/lock/lock0lock.cc:322)
>>> lock0lock.cc.o:(lock_sys_create(unsigned long)) in archive storage/innobase/libinnobase.a/usr/bin/ld.lld: error: undefined symbol: locksys::Latches::~Latches()
>>> referenced by lock0lock.h:942 (/home/mhansson/gitroot/worktrees/with_time_zone/mysql-trunk/storage/innobase/include/lock0lock.h:942)
>>> lock0lock.cc.o:(lock_sys_t::~lock_sys_t()) in archive storage/innobase/libinnobase.a
clang: error: linker command failed with exit code 1 (use -v to see invocation)
```
These versions of clang (but not 8.0.0) do not handle `Latches::~Latches = default;` declaration correctly.
Removing it does not help either.
Adding explicit `inline` keyword does not help.
What apparently helps is to provide a destructor with an empty body.
This might be somehow connected to one of these:
https://bugs.llvm.org/show_bug.cgi?id=28280
android/ndk#143
https://reviews.llvm.org/D45898
As there are minimal differences between "implicit", "defaulted" and "empty" destructor
(see https://stackoverflow.com/questions/56799129/when-to-make-a-destructor-defaulted-using-default for summary),
and certainly none that we care about here, we will cater to clang's needs and provide empty body instead of `= default`.
This patch also removes the declaration of explicitly defaulted default constructor, as simply omitting it should have same effect in our context.
Reviewed by: Nikša Skeledžija <[email protected]>
Reviewed by: Martin Hansson <[email protected]>
RB:24167
Post-push fix with workaround for clang 6.0.0 and clang 7.0.0 linking errors such as:
```
/usr/bin/ld.lld: error: undefined symbol: locksys::Latches::~Latches()
>>> referenced by lock0lock.cc:322 (/home/mhansson/gitroot/worktrees/with_time_zone/mysql-trunk/storage/innobase/lock/lock0lock.cc:322)
>>> lock0lock.cc.o:(lock_sys_create(unsigned long)) in archive storage/innobase/libinnobase.a/usr/bin/ld.lld: error: undefined symbol: locksys::Latches::~Latches()
>>> referenced by lock0lock.h:942 (/home/mhansson/gitroot/worktrees/with_time_zone/mysql-trunk/storage/innobase/include/lock0lock.h:942)
>>> lock0lock.cc.o:(lock_sys_t::~lock_sys_t()) in archive storage/innobase/libinnobase.a
clang: error: linker command failed with exit code 1 (use -v to see invocation)
```
These versions of clang (but not 8.0.0) do not handle `Latches::~Latches = default;` declaration correctly.
Removing it does not help either.
Adding explicit `inline` keyword does not help.
What apparently helps is to provide a destructor with an empty body.
This might be somehow connected to one of these:
https://bugs.llvm.org/show_bug.cgi?id=28280
android/ndk#143
https://reviews.llvm.org/D45898
As there are minimal differences between "implicit", "defaulted" and "empty" destructor
(see https://stackoverflow.com/questions/56799129/when-to-make-a-destructor-defaulted-using-default for summary),
and certainly none that we care about here, we will cater to clang's needs and provide empty body instead of `= default`.
This patch also removes the declaration of explicitly defaulted default constructor, as simply omitting it should have same effect in our context.
Reviewed by: Nikša Skeledžija <[email protected]>
Reviewed by: Martin Hansson <[email protected]>
RB:24167
What happens is so strange and very hard to reproduce and debug. Basically clang fails to export a single default destructor and I had to do this https://codereview.qt-project.org/#/c/160374/ workarround. Qt has probably hundreds or even thousands of default destructors which are exported correctly by clang, but somehow, only this one fails to export it.
Sadly, it was impossible for me to come with a simple example ... so in order to reproduce it you'll have to compile the entire Qt. Because there are a few pending patches that you need to apply first, please let me know when you want to start working on this bug to tell you the steps.
If I'll write them now, probably in a few weeks they will be outdated.
The text was updated successfully, but these errors were encountered: