version: multiple conditions
Joakim via Digitalmars-d
digitalmars-d at puremagic.com
Sat Jul 4 14:24:32 PDT 2015
On Saturday, 4 July 2015 at 15:04:32 UTC, Artur Skawina wrote:
> 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.
Well, technically, _he_ was wrong, as D defines 'linux' and not
'Linux', and I was just correcting Daniel. Note also that I
changed it to D_LP32, even though neither one is predefined,
because that at least follows the naming convention of D_LP64,
which does exist.
> 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)
> ...
A fair criticism, but one that also applies to the C++ #ifdefs I
quoted previously.
>>> 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.
Only drawback is that you have to import that config module
everywhere for those constants to be defined. I agree that
mistyping and checking a large array of version constants can
cause problems, don't think it's enough to throw "version" out
though.
More information about the Digitalmars-d
mailing list