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.
