Skip to content
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.

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

Fix compilation on DragonFlyBSD #2028

Merged
merged 3 commits into from
Jan 16, 2018
Merged

Conversation

dkgroot
Copy link
Contributor

@dkgroot dkgroot commented Jan 10, 2018

  • Also do a little formatting cleanup
  • Add static assert to prevent compilation when IP_ADD_MEMBERSHIP, IP_MULTICAST_LOOP are missing (not sure if this is preferred or not)

….netinet.in_' is fine, when compiler support is there.
@@ -77,6 +77,13 @@ version(Windows) {
}
else
import core.sys.freebsd.netinet.in_ : IP_ADD_MEMBERSHIP, IP_MULTICAST_LOOP;
} else version (DragonFlyBSD) {
static if (__VERSION__ < 2077) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be 2079 ? Since it's not merged yet, and 2078 was already released.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You raising this question, made me think about this again. As this source is only going to be compiled/used when the changes have already landed in the compiler + druntime. It does make sense to even check the compiler version. I will remove this static if and just import core.sys.dragonflybsd.netinet.in_. There really is not need for the fall back.

Copy link
Contributor

@thaven thaven left a comment

Choose a reason for hiding this comment

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

Slightly offtopic: in fact it is a shame we need all this. druntime really should have a way to get such common constants in a more OS independent way. (e.g. by providing those somewhere as aliases to their OS specific definitions)

}
else version (DragonFlyBSD)
{
static if (__VERSION__ < 2077) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Somehow the comment of @Geod24 is shown as outdated, but he is right, this should be __VERSION__ < 2079 (or even later, if core.sys.dragonflybsd doesn't get merged into druntime master before 2.079)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See
No need to check the compiler version. It's like a chicken/egg issue, without compiler support there is not way to get to this code. When compiler support lands, then core.sys.dragonflybsd.netinet.in_ will exist.

@dkgroot
Copy link
Contributor Author

dkgroot commented Jan 11, 2018

@thaven I fully agree. It would make more sense and be a lot nicer if there was an generic interface that could be referred to.

dkgroot added a commit to dkgroot-dlang/eventcore that referenced this pull request Jan 11, 2018
….netinet.in_" will work, when compiler support is there.

[See](vibe-d/vibe.d#2028 (comment))
@s-ludwig
Copy link
Member

Usually those constants actually are in druntime, but unfortunately they are just being added on a per-case basis instead of thoroughly going through each C header. I used to make druntime PR's for such missing constants, but lately I've been getting too busy and had to focus on just getting vibe.d to work.

@s-ludwig s-ludwig merged commit 88f3ec7 into vibe-d:master Jan 16, 2018
@dkgroot
Copy link
Contributor Author

dkgroot commented Jan 16, 2018

Ok understood. Thanks for merging this PR !

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.

4 participants