skypjack / uvw

Header-only, event based, tiny and easy to use libuv wrapper in modern C++ - now available as also shared/static library!
MIT License
1.87k stars 211 forks source link

Compiler unable to determine exception specification for UDPHandle (Intel C++ for windows 19.1) #209

Closed mizvekov closed 4 years ago

mizvekov commented 4 years ago

Tested with uvw-2.6.0 / libuv-v1.38 on Intel C++ compiler for windows 19.1.

Error obtained:

uvw\udp.h(122): error : cannot determine the exception specification of the default constructor due to a circular dependency
        using Handle::Handle;
        ^

  compilation aborted for uvw\udp.cpp (code 2)

This might need another workaround for a problem in the compiler.

skypjack commented 4 years ago

I don't have the ICC compiler and I can't set it on the CI afaik (do you know if it's possible with GH actions?). May I ask you to make some tests and provide a proper workaround? I've no clue of what can solve it since it's legal C++.

mizvekov commented 4 years ago

I don't now how to make it availabe on CI. I have it installed on my machine. You can get it for free from Intel, even the windows version, if you get the community supported edition of System Studio for example.

So my clue to solve it is to either delete the default constructor, or otherwise mark them noexcept explicitly. So this is my temporary workaround for it:

diff --git a/external/uvw/uvw/udp.h b/external/uvw/uvw/udp.h
index cee5f8b9..9429c65f 100644
--- a/external/uvw/uvw/udp.h
+++ b/external/uvw/uvw/udp.h
@@ -114,6 +114,7 @@ class UDPHandle final: public Handle<UDPHandle, uv_udp_t> {
     }

 public:
+    UDPHandle() = delete;

     using Membership = details::UVMembership;
     using Bind = details::UVUDPFlags;

I understand that this UDPHandle is not supposed to be default constructible, but I have not really used UDPHandle myself so I defer to you on that.

So this single fix is enough, there is only this one circular dependency in all the code that Intel C++ complains about, and it would probably have also worked to change the other objects in the hierarchy instead.

But you might consider doing this consistently: deleting all default constructors you don't need, or otherwise marking them default noexcept.

skypjack commented 4 years ago

Oh... I thought many classes have this problem. Is it only the udp handle instead? Btw the issue is quite curious, I love this kind of bugs from the compilers! :smile:

mizvekov commented 4 years ago

Yes, Just that UDP handle :)

skypjack commented 4 years ago

Nice to know. So, probably, an explicit constructor like the one in the tcp handle would solve the issue as well. Something like:

explicit UDPHandle(ConstructorAccess ca, std::shared_ptr<Loop> ref);

It replaces using Handle::Handle; in the UDPHandle class. The implementation is trivial as you can guess. I'd prefer this one in case it works. It wouldn't be a breaking change. No idea if someone ever used the default constructor of the udp handle but it would be better not to delete it if possible, just in case.

stefanofiorentino commented 4 years ago

Should not be explicit as it has 2 parameters, isn't it?

skypjack commented 4 years ago

Yeah, this is a good point actually, even though in this case explicit isn't a problem for itself, it's just useless. :smile: Btw the point isn't the explicit keyword but the fact that I'd replace the using with a properly defined constructor that does literally the same but gets around the bug in the ICC. This is because deleting the default constructor is a breaking change and I'd prefer to avoid it if possible.

mizvekov commented 4 years ago

explicit does still make sense for many parameter constructors (I'm not saying it does make sense in that case necessarily): https://en.cppreference.com/w/cpp/language/explicit

skypjack commented 4 years ago

@mizvekov can I ask you to test branch experimental? Thanks.

mizvekov commented 4 years ago

@skypjack it's good!