Fix forced non-const Address #5496
Conversation
|
Will squash 178b6cf after circleci passes |
| @trusted ptrdiff_t sendTo(const(void)[] buf, SocketFlags flags, Address to) { assert(0); } | ||
| @safe ptrdiff_t sendTo(const(void)[] buf, Address to) { assert(0); } | ||
| @trusted ptrdiff_t sendTo(const(void)[] buf, SocketFlags flags, in Address to) { assert(0); } | ||
| @safe ptrdiff_t sendTo(const(void)[] buf, in Address to) { assert(0); } |
There was a problem hiding this comment.
Your change breaks classes which inherit from Socket and override its methods (sendTo, here). This test ensures that this doesn't happen, so it should not be changed without a strong justification for breaking existing code. See issue 16514 for details.
There was a problem hiding this comment.
Isn't that issue about const methods? (not parameters)
But you are right, this PR would break the existing overrides.
Thoughts on how we could resolve this?
Its pretty prolific having to remove const all the way through my stack because one method was missing it :/ - by mistake?
There was a problem hiding this comment.
Its pretty prolific having to remove const all the way through my stack because one method was missing it :/ - by mistake?
std.socket is not const-aware because it predates const, and nobody updated it to support const until it was too late to change anything.
My suggestion is to accept for the moment that std.socket is an old module due for replacement, and to add a cast in your network code.
There was a problem hiding this comment.
I think it would be possible to add const and non-const overloads, and have the non-const version call the const version; however, this creates possible problems such as one part of the code overrriding the old non-const overload, but another part of the code calling the const overload, which is not going to call the overriding method.
There was a problem hiding this comment.
@CyberShadow my thoughts exactly. But maybe the const overloads could cast away the const?
Presuming no one has overloaded sendTo and actually expected non const - it should be safe?
There was a problem hiding this comment.
Is there a way to copy an Address?
|
Do I close? |
|
@dcousens I would like to say otherwise, but I don't see a way forward with |
|
Closing as per comments. |
5 participants
This is particularly annoying when using
sendTo, you can't provide aconst(Address)as it expects a non-constAddressclassinstance.And as far as I can tell, for no requisite.