Skip to content

Fix forced non-const Address #5496

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.

Sign up for GitHub

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

Closed
dcousens wants to merge 2 commits into from
Closed

Fix forced non-const Address #5496

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
56cc7d4
Fix forced non-const Address
dcousens Jun 19, 2017
178b6cf
(fixup) TestSocket too
dcousens Jun 19, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions std/socket.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2956,7 +2956,7 @@ public:
* Returns: The number of bytes actually sent, or $(D Socket.ERROR) on
* failure.
*/
ptrdiff_t sendTo(const(void)[] buf, SocketFlags flags, Address to) @trusted
ptrdiff_t sendTo(const(void)[] buf, SocketFlags flags, in Address to) @trusted
{
static if (is(typeof(MSG_NOSIGNAL)))
{
Expand All @@ -2972,7 +2972,7 @@ public:
}

/// ditto
ptrdiff_t sendTo(const(void)[] buf, Address to)
ptrdiff_t sendTo(const(void)[] buf, in Address to)
{
return sendTo(buf, SocketFlags.NONE, to);
}
Expand Down Expand Up @@ -3522,8 +3522,8 @@ class UdpSocket: Socket
@property @trusted Address localAddress() { assert(0); }
@trusted ptrdiff_t send(const(void)[] buf, SocketFlags flags) { assert(0); }
@safe ptrdiff_t send(const(void)[] buf) { assert(0); }
@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); }
Copy link
Member

@CyberShadow CyberShadow Jun 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@dcousens dcousens Jun 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

@CyberShadow CyberShadow Jun 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

@CyberShadow CyberShadow Jun 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@dcousens dcousens Jun 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Contributor Author

@dcousens dcousens Jun 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to copy an Address?

@trusted ptrdiff_t sendTo(const(void)[] buf, SocketFlags flags) { assert(0); }
@safe ptrdiff_t sendTo(const(void)[] buf) { assert(0); }
@trusted ptrdiff_t receive(void[] buf, SocketFlags flags) { assert(0); }
Expand Down