version: multiple conditions

[Artur Skawina via Digitalmars-d]

On 06/28/15 16:21, Joakim via Digitalmars-d wrote:
> On Sunday, 28 June 2015 at 13:43:39 UTC, Artur Skawina wrote:
>> On 06/28/15 05:06, Joakim via Digitalmars-d wrote:
>>> So you're trying to avoid writing this?
>>>
>>> version(linux) version(D_LP32)
>>>     version = valistIsCharPointer;
>>> else version(Windows)
>>>     version = valistIsCharPointer;
>>>
>>> While your version is more concise, I actually think the current form is more clear.
>>
>> And wrong. Spot the bug.
> 
> Eh, that idiom is not commonly used so I wasn't exactly clear on the syntax, but you're presumably referring to the need for braces?

I didn't even look as far as the 'else', but that is another good
reason to avoid 'version'.
What I was referring to was the use of subtly different identifiers
- the original used 'Linux', but your version has 'linux'. Such
changes are hard to see even in a four-liner; they are almost
impossible to catch in a larger change. Which will likely pass code
review, where the focus in on logic, not spelling. The compiler is
supposed to catch the latter kind of errors, but it has no chance to
do this because of D's 'version' semantics. So performance- and even
security-relevant issues can remain unnoticed, until someone has to
figure out why something that can't happen does happen, or why a
platform feature isn't being used. The situation is bad enough when
dealing with predefined identifiers, but gets worse with local user-
defined ones. You might get 'Linux' vs 'linux' right, but is it
'freebsd', 'freeBSD', 'FreeBSD' or 'Free_BSD'? Unless you happen to
use that OS, you /will/ get it wrong.
When reviewing code, will you always spot 'version(ALPHA)',
'version(Sparc)', 'version(x86_64)' and 'version(MIPS_64)'?
You're even less likely to notice:

   version(vaListIsCharPointer)
     ...

>> That's the real reason why 'version' needs to be banned from the codebase. You should *never* use D's 'version'.

D has another feature that provides the same functionality and
is not affected by the problem, so just use that -- 'static if'.
And have just one exception; a 'config' module which is allowed
to access predefined version identifiers (but does not define
any local ones, just normal enum constants). Not perfect, but at
least this issue will not bite you anywhere else. If someone writes

   static if (config.Linux) ...

or 

   static if (config.vaListIsCharPointer) ...

then the compiler will always catch the bug. The other D-version
problems mentioned in this thread will then disappear too.

artur