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

popo/user_trigger.hpp:33:5: error: exception specification of explicitly defaulted default constructor does not match the calculated one #494

Closed
BH1SCW opened this issue Jan 15, 2021 · 22 comments · Fixed by #490 or #531
Assignees
Labels
bug Something isn't working

Comments

@BH1SCW
Copy link
Contributor

BH1SCW commented Jan 15, 2021

Required information

Operating system:
Mac OS Mojave

Compiler version:
Apple clang version 11.0.0 (clang-1100.0.33.12)
Target: x86_64-apple-darwin18.6.0

Observed result or behaviour:
iceoryx/iceoryx_posh/include/iceoryx_posh/popo/user_trigger.hpp:33:5: error: exception specification of explicitly defaulted default constructor does not match the calculated one
UserTrigger() noexcept = default;
^
1 error generated.
gmake[2]: *** [posh/CMakeFiles/iceoryx_posh.dir/build.make:577: posh/CMakeFiles/iceoryx_posh.dir/source/popo/user_trigger.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:384: posh/CMakeFiles/iceoryx_posh.dir/all] Error 2
gmake: *** [Makefile:172: all] Error 2

Expected result or behaviour:
no build error

Conditions where it occurred / Performed steps:
git commit id:
commit f1c4b31

@mossmaurice mossmaurice added the bug Something isn't working label Jan 15, 2021
@elfenpiff
Copy link
Contributor

I did a first investigation and where unable to reproduce the bug on
OS: Mac OS Big Sure
Compiler: Apple clang version 12.0.0 (clang-1200.0.32.28)

@BH1SCW would it be possible for you to switch for now to the latest Mac OS Version with the latest clang compiler?

@sculpordwarf
Copy link
Contributor

@elfenpiff
I´ve seen this problem in Ubuntu 20.04 some time ago, too. Using the 'noexpect = default;' constructor seems to cause problems on different compilers. From my point of view, I would go for the compatible style in the code base. Maybe in some years it can be changed back.
'class MyClass { public: MyClass() noexcept {}}'

@BH1SCW
Copy link
Contributor Author

BH1SCW commented Jan 18, 2021

I did a first investigation and where unable to reproduce the bug on
OS: Mac OS Big Sure
Compiler: Apple clang version 12.0.0 (clang-1200.0.32.28)

@BH1SCW would it be possible for you to switch for now to the latest Mac OS Version with the latest clang compiler?

Our company's vpn software has compatibility issues with the latest mac os, that‘s the reason why I still not upgrade my OS.

elBoberido pushed a commit to ApexAI/iceoryx that referenced this issue Jan 19, 2021
…r msvc and mac

Signed-off-by: Christian Eltzschig <me@elchris.org>
@elBoberido elBoberido linked a pull request Jan 19, 2021 that will close this issue
19 tasks
@elBoberido
Copy link
Member

With the #490 PR this specific error should be fixed.

There are about 30 further usages of Ctor()/Dtor() noexcept = default;. The question is, shall we also implement the ctor/dtor for these or do we wait and do it on a case by case basis?

@mossmaurice
Copy link
Contributor

The question is, shall we also implement the ctor/dtor for these or do we wait and do it on a case by case basis?

Are all those occurences leading to compile errors? I'm still struggeling to understand what causes the error. If a c'tor has noexcept in its signature, it can't be implicitly declared?

@elBoberido
Copy link
Member

elBoberido commented Jan 20, 2021

The question is, shall we also implement the ctor/dtor for these or do we wait and do it on a case by case basis?

Are all those occurences leading to compile errors? I'm still struggeling to understand what causes the error. If a c'tor has noexcept in its signature, it can't be implicitly declared?

From what I understand, some compiler have sometimes problems with it.

Maybe it's similar to defaulting a copy/move ctor. If there is a member which has no copy/move ctor, defaulting implicitly means deletion. In this case it might be the same with members without ctors with the noexcept specifier.

@sculpordwarf
Copy link
Contributor

@mossmaurice At least it was a problem on my side with the 20.04 some time ago. I just changed the compiled stuff only, but for the master it needs to be everywhere corrected (else somebody builds e.g. a test and it fails again).

constructor does not match the calculated one
UserTrigger() noexcept = default;

If you remove the noexpect its gone or if you define the constructor empty.

@elBoberido
Copy link
Member

Okay, I did a small test. If you have a member of a class with MemberCtor() noexcept(false) = default; the std::is_nothrow_default_constructible/std::is_nothrow_constructible traits evaluate to false for the class with that member, so my assumption was correct and the ctor gets deleted.
I don't know it it makes sense to add this check to our tests, since it won't compile anyway if the ctor is deleted.
This seems also highly compiler/STL version specific. There might be only two viable options, like @sculpordwarf already pointed out.
With the removal of the noexcept specifier we could still create a test for std::is_nothrow_default_constructible to check if it's implicitly noexcept, but I have a slight leaning towards implementing an empty default constructor.

@elfenpiff
Copy link
Contributor

...but I have a slight leaning towards implementing an empty default constructor.

I vote for implementing an empty default ctor!

@elBoberido
Copy link
Member

...but I have a slight leaning towards implementing an empty default constructor.

I vote for implementing an empty default ctor!

on the other hand, this would also apply to defaulted copy/move ctors and we were not anymore able to do the rule of zero since a class without a ctor will implicitly be nothrowable except if it has a member which does throw, then it's implicitly throwable ... my head just banged on the table 🤕

@mossmaurice
Copy link
Contributor

on the other hand, this would also apply to defaulted copy/move ctors and we were not anymore able to do the rule of zero since a class without a ctor will implicitly be nothrowable except if it has a member which does throw, then it's implicitly throwable ... my head just banged on the table face_with_head_bandage

Are you sure? Do you have a Godbolt snippet for that?

According to here and here, they even went back and forth in the committee 😒

If the user explicitly specifies an exception specification on a defaulted function, that’s the exception specification. Don’t delete the function, don’t reject the program, just accept it.

I suppose we should check if we can fix the clang build by changing the build to C++17 or adding some magic compiler option which works as written above.

@elfenpiff
Copy link
Contributor

...but I have a slight leaning towards implementing an empty default constructor.

I vote for implementing an empty default ctor!

on the other hand, this would also apply to defaulted copy/move ctors and we were not anymore able to do the rule of zero since a class without a ctor will implicitly be nothrowable except if it has a member which does throw, then it's implicitly throwable ... my head just banged on the table

Just a short recap.

  1. We do not use or support exceptions anywhere (ensured through noexcept).
  2. A ctor() noexcept = default gets deleted if a member has a default ctor ctor() noexcept(false) which should be never the case in our code.

Therefore, we should use ctor() noexcept = default; and never implement it empty our selfs and when the problem arises we have an issue in a member which throws an exception in the ctor which is not allowed! Hence, not doing this and implementing it empty our self would hide the exception throwing ctor which would be a serious issue.

@elBoberido
Copy link
Member

elBoberido commented Jan 20, 2021

I don't have a godbolt snippet, but you can run this locally

#include <iostream>
#include <type_traits>

struct Bar {
    Bar() noexcept(false) = default;

    Bar(const Bar&) noexcept(false) = default;
    Bar(Bar&&) noexcept(false) = default;
};

struct Foo {
    Foo() noexcept = default;

    Foo(const Foo&) noexcept = default;
    Foo(Foo&&) noexcept = default;

    Bar b;
};
int main() {
    std::cout << "Is nothrow default constructible: " << std::is_nothrow_default_constructible<Foo>::value << std::endl;
    std::cout << "Is nothrow move constructible   : " << std::is_nothrow_move_constructible<Foo>::value << std::endl;
}

@elfenpiff I also came to the same conclusion, but what if the class is from the STL, e.g. atomic (I don't know if that's really the case, just as theoretical example)

@elfenpiff
Copy link
Contributor

@elBoberido you are right but then we would have to write our own atomic since we explicitly do not allow any kind of exception. But I think the atomic does throw nothing so we should be safe in this case.

@elBoberido
Copy link
Member

@elfenpiff the atomic was just an (maybe bad) example. The problem is that some types of the STL might cause the deletion of a defaulted noexcept ctor. That's out of reach for us to fix.

@elBoberido
Copy link
Member

@elfenpiff I just found an example for an STL type which causes the deletion on a defaulted noexcept ctor when used as member

std::chrono::duration<int64_t, std::nano> d{0};

@elfenpiff
Copy link
Contributor

@elfenpiff I just found an example for an STL type which causes the deletion on a defaulted noexcept ctor when used as member

std::chrono::duration<int64_t, std::nano> d{0};

And with this std::chrono as duration alternative is gone ...

@elBoberido
Copy link
Member

@BH1SCW can you check if the current master works for you?

We need to find a proper solution to prevent this breakage in the future, so I would keep this issue open

@BH1SCW
Copy link
Contributor Author

BH1SCW commented Jan 22, 2021

@BH1SCW can you check if the current master works for you?

We need to find a proper solution to prevent this breakage in the future, so I would keep this issue open

I just checked, current master is ok.

@dkroenke
Copy link
Member

We need to find a proper solution to prevent this breakage in the future, so I would keep this issue open

MacOS Mojave (10.14) is available as image from GitHub CI.

Question is, do we need to support also the older versions of MacOS? I guess in future it can happen that the container is removed then. I would also vote against an additional build because the worker for MacOS are more limited than Ubuntu: https://docs.github.com/en/github/setting-up-and-managing-billing-and-payments-on-github/about-billing-for-github-actions.

One possibility is to install the older clang version on the actual MacOS runner and have an additional build step to compile with it.

@elBoberido
Copy link
Member

@dkroenke I think we have to check what is needed to become Tier1 in ROS2 and align with that.

@dkroenke
Copy link
Member

According to the current https://www.ros.org/reps/rep-2000.html for ROS2 Foxy and Galactic is MacOS Mojave (10.14) needed.
I guess we can then tie the MacOS runners to that version. I can do it within this ticket here.

It can happen that in future MacOS support is downgraded to Tier3: https://github.com/ros-infrastructure/rep/pull/291/files But also there is Mojave mentioned.

dkroenke added a commit to ApexAI/iceoryx that referenced this issue Jan 29, 2021
Signed-off-by: Dietrich Krönke <dietrich.kroenke@apex.ai>
dkroenke added a commit to ApexAI/iceoryx that referenced this issue Jan 29, 2021
Signed-off-by: Dietrich Krönke <dietrich.kroenke@apex.ai>
dkroenke added a commit to ApexAI/iceoryx that referenced this issue Jan 29, 2021
…ithub-CI

Signed-off-by: Dietrich Krönke <dietrich.kroenke@apex.ai>
dkroenke added a commit to ApexAI/iceoryx that referenced this issue Jan 29, 2021
Signed-off-by: Dietrich Krönke <dietrich.kroenke@apex.ai>
dkroenke added a commit that referenced this issue Jan 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment