Skip to content

std.socket: Fix some -dip1000 compilable issues #6204

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
carblue wants to merge 1 commit into from
Closed

std.socket: Fix some -dip1000 compilable issues #6204

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 12 additions & 12 deletions std/socket.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2517,13 +2517,13 @@ public:
assert(!errorSet.isSet(testPair[1]));

ubyte[1] b;
testPair[0].send(b[]);
() @safe { testPair[0].send(b[]); }();
fillSets();
n = Socket.select(readSet, null, null);
assert(n == 1); // testPair[1]
assert(readSet.isSet(testPair[1]));
assert(!readSet.isSet(testPair[0]));
testPair[1].receive(b[]);
() @safe { testPair[1].receive(b[]); }();
}
});
}
Expand Down Expand Up @@ -2929,7 +2929,7 @@ public:
* Calling shutdown() before this is recommended
* for connection-oriented sockets.
*/
void close() @trusted nothrow @nogc
void close() @trusted nothrow @nogc scope
{
_close(sock);
sock = socket_t.init;
Expand Down Expand Up @@ -2993,7 +2993,7 @@ public:
* Returns: The number of bytes actually sent, or $(D Socket.ERROR) on
* failure.
*/
ptrdiff_t send(const(void)[] buf, SocketFlags flags) @trusted
ptrdiff_t send(const(void)[] buf, SocketFlags flags) scope @trusted
{
static if (is(typeof(MSG_NOSIGNAL)))
{
Expand All @@ -3007,7 +3007,7 @@ public:
}

/// ditto
ptrdiff_t send(const(void)[] buf)
ptrdiff_t send(const(void)[] buf) scope
{
return send(buf, SocketFlags.NONE);
}
Expand Down Expand Up @@ -3070,7 +3070,7 @@ public:
* Returns: The number of bytes actually received, $(D 0) if the remote side
* has closed the connection, or $(D Socket.ERROR) on failure.
*/
ptrdiff_t receive(void[] buf, SocketFlags flags) @trusted
ptrdiff_t receive(void[] buf, SocketFlags flags) scope @trusted
{
version(Windows) // Does not use size_t
{
Expand All @@ -3087,7 +3087,7 @@ public:
}

/// ditto
ptrdiff_t receive(void[] buf)
ptrdiff_t receive(void[] buf) scope
{
return receive(buf, SocketFlags.NONE);
}
Expand Down Expand Up @@ -3582,17 +3582,17 @@ class UdpSocket: Socket
protected pure nothrow @safe Socket accepting() { assert(0); }
@trusted Socket accept() { assert(0); }
nothrow @nogc @trusted void shutdown(SocketShutdown how) { assert(0); }
nothrow @nogc @trusted void close() { assert(0); }
nothrow @nogc @trusted void close() scope { assert(0); }
@property @trusted Address remoteAddress() { assert(0); }
@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 send(const(void)[] buf, SocketFlags flags) scope { assert(0); }
@safe ptrdiff_t send(const(void)[] buf) scope { 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) { assert(0); }
@safe ptrdiff_t sendTo(const(void)[] buf) { assert(0); }
@trusted ptrdiff_t receive(void[] buf, SocketFlags flags) { assert(0); }
@safe ptrdiff_t receive(void[] buf) { assert(0); }
@trusted ptrdiff_t receive(void[] buf, SocketFlags flags) scope { assert(0); }
@safe ptrdiff_t receive(void[] buf) scope { assert(0); }
Copy link
Member

@CyberShadow CyberShadow Feb 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

You cannot change this class. It is part of a unit test, so by changing it, you are changing a test that has been put in place to guard against breaking user code. This class is here to test for inadvertently breaking user classes which derive from Socket. Any changes here need to have a strong rationale for potentially breaking user code.

Copy link
Contributor Author

@carblue carblue Feb 21, 2018

Choose a reason for hiding this comment

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

There seems to be a conflict of objectives:
To the best of my knowledge - with current DIP1000.md and it's implementation (scope is a part of (member) function's signature/mangling) - I don't see a way to have std.socket -dip1000 compilable without breaking user code (i.e. change class Socket). According to https://github.com/dlang/DIPs/blob/master/DIPs/DIP1000.md#breaking-changes--deprecation-process it's known there will be breaking changes; IMHO this PR is an example for coming breakages, but I now understand - supposed my PR is correct - it shouldn't be merged without a deprecation process in concert with -transition=safe.

Copy link
Contributor

@wilzbach wilzbach Feb 23, 2018

Choose a reason for hiding this comment

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

CC @andralex @MartinNowak @WalterBright
What's the strategy for such errors resulting from DIP1000 upgrades?
Do we need to introduce some @__future like behavior in the compiler to issue deprecations?

Copy link
Member

@MartinNowak MartinNowak Feb 23, 2018

Choose a reason for hiding this comment

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

It simply means that Socket classes and sub-classes cannot be used as scoped classes with DIP1000, because it's unclear whether they escape sth. or not. It should remain fully useable with GC allocated Socket classes though.
As @CyberShadow said, if Socket is not final we cannot suddenly require all derivatives to obey to not escaping sth.
That's unfortunate but a big restriction with open interface&classes in APIs.
I asked for @future to become useful for such deprecations as well, but it made it to a mentioning in https://github.com/dlang/DIPs/blob/master/DIPs/DIP1007.md#user-content-proposal under 6..
I don't think exempting Socket from DIP1000 is a big issue, the whole module is outdated (by todays D standards) and class based sockets are questionable. Here is a possible replacement https://github.com/MartinNowak/io (though I still need to get return scope correct).

Copy link
Contributor Author

@carblue carblue Feb 25, 2018

Choose a reason for hiding this comment

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

There is no two-tier memory safety.
D can't ever claim memory safety of @safe code, if it starts to exempt some phobos module(s) from memory checks which possibly may then escape pointers to expired stack frames or alike.
Walter said at DConf2017, he believes there is a tsunami coming and predicted, it will kill the C language. That's my opinion too ref. growing importance of memory safety for language selection.
And D still isn't ahead of the caravan. https://www.youtube.com/watch?v=Lo6Q2vB9AAg

Was there a survey, whether users put up with code breakage mitigated by a reasonable deprecation duration, when they get memory safety in return?

Copy link
Contributor Author

@carblue carblue Mar 7, 2018

Choose a reason for hiding this comment

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

I take this as PR rejection and will close it

Copy link
Member

@WalterBright WalterBright Mar 28, 2018

Choose a reason for hiding this comment

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

How about simply making std.socket @System so it will compile with -dip1000 and move on?

Copy link
Contributor Author

@carblue carblue Mar 28, 2018

Choose a reason for hiding this comment

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

Yes!! It's as simple as that to avoid changing class socket and be -dip1000 compilable

Copy link
Member

@WalterBright WalterBright Mar 29, 2018

Choose a reason for hiding this comment

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

Let's do it, then.

Copy link
Contributor

@wilzbach wilzbach Mar 29, 2018

Choose a reason for hiding this comment

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

-> #6384

@trusted ptrdiff_t receiveFrom(void[] buf, SocketFlags flags, ref Address from) { assert(0); }
@safe ptrdiff_t receiveFrom(void[] buf, ref Address from) { assert(0); }
@trusted ptrdiff_t receiveFrom(void[] buf, SocketFlags flags) { assert(0); }
Expand Down