Skip to content

Comments

Eliminate the need for shared static this() on Posix#7529

Closed
andralex wants to merge 2 commits intodlang:masterfrom
andralex:socket
Closed

Eliminate the need for shared static this() on Posix#7529
andralex wants to merge 2 commits intodlang:masterfrom
andralex:socket

Conversation

@andralex
Copy link
Member

It would be great if we could eliminate all static ctors/dtors from Phobos and use lazy initialization instead. This PR is a simple step in that direction - there's really no need to initialize those pointers to function (or test them) in Posix.

@andralex andralex requested a review from CyberShadow as a code owner June 14, 2020 18:42
@dlang-bot
Copy link
Contributor

Thanks for your pull request, @andralex!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + phobos#7529"

std/socket.d Outdated
{
enum getnameinfoPointer = &getnameinfo;
enum getaddrinfoPointer = &getaddrinfo;
enum freeaddrinfoPointer = &freeaddrinfo;

Choose a reason for hiding this comment

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

Remove these for Posix, add version(Windows) { ... } else version(Posix) { ... } else static assert(0); where applicable to avoid un-necessary indirections.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I'll add the assert. To get rid of the enums I'd need to wait for the updated Windows version.

Copy link
Member Author

Choose a reason for hiding this comment

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

Update: I see there's a static assert on line 126, so no need to duplicate that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just checked, only dmd generates actual indirections :). https://godbolt.org/z/RmDYYV

Copy link
Member Author

Choose a reason for hiding this comment

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

Even better: I consolidated those to the top version blocks.

@andralex
Copy link
Member Author

I had flashbacks that I worked on that before... and indeed I did: #5813. Let's continue there.

@andralex andralex closed this Jun 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants