unnecessary OS redundancy in druntime

[Martin Nowak via Digitalmars-d]

On 12/12/2014 04:47 PM, Joakim wrote:
> I asked about this on github but didn't get a good answer, so I'm asking
> here.  What's with all the repeated OS blocks in druntime?

No, you don't want to accept the answer. That's slightly different than 
not getting none.

> https://github.com/D-Programming-Language/druntime/blob/master/src/core/sys/posix/unistd.d#L945
>
> https://github.com/D-Programming-Language/druntime/blob/master/src/core/sys/posix/netinet/in_.d#L974
>
> https://github.com/D-Programming-Language/druntime/blob/master/src/core/sys/posix/arpa/inet.d#L201
>
>
> It seems like pointless repetition and there are many more examples of
> this, as I keep running into it when adding bionic/Android support.
> Martin suggested that it would be useful to have a default case that
> asserts for an unsupported OS, but many of these blocks don't have that
> either.

Because it was written before we spread out to multiple architectures, 
and learned hard it is to add something.

>
> Why not just declare them once for Posix and then specialize for a
> particular OS later on when necessary, as seems to have been done in
> these instances?

We've been through this several times, because the poor guy adding 
support for a new OS or arch has the find all the differences through 
debugging.

> As can be seen from these links, druntime is not consistent in how it's

Have you ever seen software with more than a 100 LOC that's totally 
consistent? There have been many contributors, the project is several 
years old...

 > If we can hash out how it should be done, I'll submit a pull to
> fix it up.

There nothing unclear here.

version (A)
else version (B)
else static assert(0, "unimplemented");

As Daniel pointed out, if you run into something that's extremely want 
to save some you might factor out common stuff into a default block.

version (A)
     import ...default;
else version (B)
     import ...default;
else version (C)
     something else
else
     static assert(0, "unimplemented");

That's already problematic for a reviewer, because it's much harder to 
check the correctness of a patch (when comparing it with the C headers).