[Silverclaw]

I've been coding D and I used Derelict for a few years already on my projects, specially the GL and SDL packages. I also have suicidal tendencies and as such am using DMD2.

Theres been a while since I've coded GL and now that i'm getting back to it, i came here to get an updated version of the bindings. Since D2 introduces TLS I've read the related forum topics regarding this issue and peeked through the code on derelict. While doing it, i came across some of the developer's design decisions I didnt understand.

Firstly, the enourmous ammounts of mixins. True, they are all beign used correctly, but there some that I don't understand why they are there. For example, consider the next code excert from compat.d:

version(D_Version2)
{
mixin("alias const(char)* CCPTR;");
mixin("alias const(wchar)* CWCPTR;");
mixin("alias const(dchar)* CDCPTR;");
mixin("alias const(ubyte)* CUBPTR;");
mixin("alias const(void)* CVPTR;");
}

I don't understand why "mixin" is necessary here. This code should be equivalent to

version(D_Version2)
{
alias const(char)* CCPTR;
alias const(wchar)* CWCPTR;
alias const(dchar)* CDCPTR;
alias const(ubyte)* CUBPTR;
alias const(void)* CVPTR;
}

Obviously, I must've missed something.


Next, usage of version'ed functions is used accross the packages, however, other functions that should be aliased, are not. Consider:

string stripWhiteSpace(string str)
{
version(Tango)
{
return trim(str);
}
else
{
return strip(str);
}
}

Why not

version(Tango)
{
alias trim stripWhiteSpace;
}
else
{
alias strip stripWhiteSpace;
}

Which has the advantage of not created a pass-function to the linker. Granted, any decent compiler will optimize it away, but the solution used is a waste of the language's resources.


The loader is currently a global reference to an object that is constructed on the static this(). While this is ok, the reference itself is not shared accross threads (its set on TLS). Consider the declaration at line 883 in gl.d:

DerelictGLLoader DerelictGL;

My question is, why not implement DerelictGLLoader as a singleton? Its own private reference could then be set to a final shared reference (which doesnt require synchronization since i can't be modified) created on the private this(). To initialize on the static this() one would only need "new DerelictGLLoader");


Lastly, the abstract class SharedLibLoader recieves 3 parameters on its contructor, one for each platform that can be linked with the current version. I don't see the reasoning behind this. The version() statements should be moved to the inherited classes and only 1 parameter should be recieved by the abstract class. My rationale is that if one would want to add another architecture, the base class would require a new argument on its list, requiring all other classes to be modified accordingly. The opposite is also problematic. If one would want to remove the link libraries for a platform the loader would be given a null string for that platform. Moving the version statements to the base classes would allow the developers to add static assert(false, "platform not supported");


Sorry for the long winded post. I'm sure you have good reasons to have designed it the way you did. But if I am to make any patches for the libraries, I would need to know why it was designed the way it did. I do not wish to break any traditions.

[doob]

I can answer one of your questions.

[Silverclaw]

[doob]

[Stanley Pancakes]

Well, there is 'another' solution if it may be considered as such:

[doob]

[aldacron]

[Silverclaw]

[aldacron]

[aldacron]

[Silverclaw]

I understand the reasoning, alas I simply wanted to reduce the number of string literals the compiler has to handle (and therefore less manifest constants), less stack allocations due to parameter passing (which as I said before, compilers *should* be smart enough to remove).

Although, since both loaders aren't mutually exclusive, would it be possible to keep at least the single-string constructor of SharedLoader?


On a side note, there is little documentation spread on the source. Is this on purpose? (ie, is the source documentation found elsewhere?) If not, I might add a doc comment here and there as I've lost my way around the code a couple of times already.

[aldacron]