Why version() ?

[Walter Bright]

Steven Schveighoffer wrote:
> "Walter Bright" wrote
>> 2. version (A || B) can be done as:
>>
>>     version (A) version=AorB;
>>     version (B) version=AorB;
> 
> And it would be much nicer NOT to have to do this.

If you find yourself doing that a lot in the code, then I suggest a 
better solution is to rethink exactly which versions are being produced, 
and come up with a set of version identifiers, one for each actual version.

> I think your original 
> position of "let's not get into #if hell" has absolutely (and I mean 
> *absolutely*) no merit as an argument against this one change.

The defense submits as Exhibit A just one snippet from Hans Boehm gc's 
os_dep.c (note all it uses in #if expressions is ||):

===============================
# if defined(NEED_FIND_LIMIT) || defined(UNIX_LIKE)

#   ifdef __STDC__
         typedef void (*handler)(int);
#   else
         typedef void (*handler)();
#   endif

#   if defined(SUNOS5SIGS) || defined(IRIX5) || defined(OSF1) \
     || defined(HURD) || defined(NETBSD)
         static struct sigaction old_segv_act;
#       if defined(IRIX5) || defined(HPUX) \
         || defined(HURD) || defined(NETBSD)
             static struct sigaction old_bus_act;
#       endif
#   else
         static handler old_segv_handler, old_bus_handler;
#   endif

#   ifdef __STDC__
       void GC_set_and_save_fault_handler(handler h)
#   else
       void GC_set_and_save_fault_handler(h)
       handler h;
#   endif
     {
#       if defined(SUNOS5SIGS) || defined(IRIX5)  \
         || defined(OSF1) || defined(HURD) || defined(NETBSD)
           struct sigaction      act;

           act.sa_handler        = h;
#         if 0 /* Was necessary for Solaris 2.3 and very temporary      */
                /* NetBSD bugs.                                          */
             act.sa_flags          = SA_RESTART | SA_NODEFER;
#         else
             act.sa_flags          = SA_RESTART;
#         endif

           (void) sigemptyset(&act.sa_mask);
#         ifdef GC_IRIX_THREADS
                 /* Older versions have a bug related to retrieving and  */
                 /* and setting a handler at the same time.              */
                 (void) sigaction(SIGSEGV, 0, &old_segv_act);
                 (void) sigaction(SIGSEGV, &act, 0);
                 (void) sigaction(SIGBUS, 0, &old_bus_act);
                 (void) sigaction(SIGBUS, &act, 0);
#         else
                 (void) sigaction(SIGSEGV, &act, &old_segv_act);
#               if defined(IRIX5) \
                    || defined(HPUX) || defined(HURD) || defined(NETBSD)
                     /* Under Irix 5.x or HP/UX, we may get SIGBUS.      */
                     /* Pthreads doesn't exist under Irix 5.x, so we     */
                     /* don't have to worry in the threads case.         */
                     (void) sigaction(SIGBUS, &act, &old_bus_act);
#               endif
#         endif /* GC_IRIX_THREADS */
#       else
           old_segv_handler = signal(SIGSEGV, h);
#         ifdef SIGBUS
             old_bus_handler = signal(SIGBUS, h);
#         endif
#       endif
     }
# endif /* NEED_FIND_LIMIT || UNIX_LIKE */
===============================

The rest of that file is ALL like that. That one little innocuous change 
opens the door to hell <g>.

(Disclaimer: I do not mean to throw tomatoes specifically at Hans here, 
I know Hans personally and I am in awe of his programming knowledge 
skill. It's just that many developers have worked on that gc, each one 
layering on another gob of conditional compilation. This result is 
typical of C code that's been maintained for years, and you'll find a 
similar mess in my own code. I didn't want to use my own code as the 
example of hell because that is too easily dismissed as a personal 
failing of mine, and that professionals wouldn't do that. But they do. C 
lends itself to and encourages this kind of programming.)

> 
> Think of code that is versioned around architecture that would look 
> horrendous if you have to do version statements that have all different 
> combinations of stuff.  If I have 5 different architectures to support, I 
> don't want to have to define a module that has 2^5 different version 
> combinations.

Is your code really intended to actually build 32 different versions? 
May I suggest instead building the 5 architecture versions, and using 
runtime switches for the variants? Or perhaps abstracting the particular 
features out into separate modules?


> When I add another architecture, *gasp* I have to double the statements (to 
> do them now with and without version(F) ), and now I have to do another 2^5 
> statements for the version(F) block.  Wheee!

You're right, that is untenable. What I suggest instead is creating a 
module:

    module PersonalityForF;

with all the oddities for F exported as constants, aliases, types and 
functions. This makes it easier for the maintenance developer who needs 
to do G, he knows he's just got to write a PersonalityForG module rather 
than edit the source code for dozens of other modules that have embedded 
version stuff. It also eliminates the risk of breakage of those other 
modules for the other platforms. (When I do a port of my projects to new 
platform F, I often inadvertently break A and C because the code for A, 
C and F are all mixed up together.)

I'm currently working on the Mac port, and am encountering exactly these 
kinds of problems. One bright spot is I abstracted all the platform 
specific library reading/writing code out into a separate file rather 
than using any conditional compilation. This has worked out extremely 
well. No "os_dep.c" disaster file.



>> 5. Why can't one 'version out' syntax that is not recognized by the 
>> compiler?
>>
>> The problem is that supporting this requires semantic analysis in order to 
>> successfully lex and parse the source code. Breaking this will make the 
>> lexing and parsing an order of magnitude harder for third party tools to 
>> do. If you need to 'comment out' a section of syntactically invalid code, 
>> use the /+ ... +/ nesting comment.
> 
> Just so you know, this is not a solution.  We all know that the main reason 
> people ask for this is to have code that can compile with D1 or D2 using 
> versioning.  /+ .. +/ doesn't help there.

I know, and I have the same issues with Phobos. See my reply to Derek.