Macro abuse?

Help
Alex
2007-04-06
2013-04-25
  • Alex

    Alex - 2007-04-06

    Hi,

    I read the FAQ called 'Compiling hangs', and decided to check out those files. Wow! That's a lot of macro abuse. This is a C++ project? Why do you need to do such dangerous/unmaintainable things? Does your compiler still not support templates or something?

    If you're so worried about incurring an additional function call, you can always inline it. But I doubt the benefits would be as significant as you might think.

    I really think all those macro's lead to hugely unmaintainable code. Please explain why you *really* need to do such a thing.

    And 20-30 minutes compilation time for 1 file (on a fast machine) is insane!

     
    • Okan Arikan

      Okan Arikan - 2007-04-06

         Dear Alex,

         We really appreciate constructive criticisms. However, I would really look into the code and actually see what these macros are doing that no inline expansion or template instantiation can do.

         If you see any alternatives that is of comparable performance, please let me know.

         Okan  

       
      • George Harker

        George Harker - 2007-04-06

        Hi Alex,

        I'd like to second Okan's response.

        From time to time, this question comes up.  By way of some explanation, the function of the macros is not so much to inline code, as to textually construct functions.  This is something which templates are very poor at (they weren't designed for it).  Templates are a benefit when the same algorithm can be reusued with multiple types.  Certain aspects of the algorithm may be made generic via template arguments.  However, constructing functions this way requires functor classes and is to my mind rather unnatural.  Inlining functions doesn't particularly help as it would place a large burden on the developer to maintain the common looping constructs - which is error prone.

        Consider some of the function-building macros.  They take a common looping construct and fill in the before (pre), in-loop operation, post-operation update and post-loop code.  This could be written as:

        macro:

        makeFunc(pre,inner,update,post)\ pre();\ for(loop construct){\    if(cond) inner();\    update();\ }\ post();

        and I fill in pre,post etc when instantiating. It is true that pre,post etc are not typechecked - that is because they are essentially _typeless_.  They are simply blocks of text.

        functors / templates:

        I must re-write each of the ops as a class so it can be passed to my template:

        struct pre{
        void operator() (....all the args we need, lots...) { }
        };

        struct post{
        void operator() (....all the args we need, lots...) { }
        };
        ....

        template<class _pre,class _inner,class _update,class _post>
        class makeFunc{
          makeFunc (pre,inner,update,post)
          _pre pre;
          _inner inner;
          _update update;
          _post  post;

          pre();
          for(loop construct){
              if(cond) inner();
              update();
          }
          post();
        }
        };

        or something like it, which is messy.  We must create tonnes of stub classes to wrap code up so that they can be passed as template arguements.  Plus the verbosity is greater.  If we had to do this, I would probably want to use macros to save typing!

        The functions actually have to take a lot of arguments which complicates things greatly.

        Furthermore, there are times when the looping construct must itself be conditionalized by pre() and post() - ie these contain an open conditional which wraps the whole loop.  I'd find myself jumping through greater and greater hoops to do that with templates and classes - all of which is unnecessary baggage to get the job done.

        Finally, there's the inline-only solution. 

        Perhaps we should make each SL function it's own shadeop:

        MySLFunc(....lots of args....) {
        pre();
        for(loop construct){
           if(cond) inner();
           update();
        }
        post();
        }

        Which looks the same as before, only now we have tonnes of looping and conditional constructs to maintain.

        If anything the macro system we are using is to keep common parts of the code together and increase maintainablility.  For example, should we need to alter the looping constructs, there are a limited (3-4) places in the code we must touch.  Their use is not dangerous, but well considered and necessary.

        Incidentally, there has never in the history of Pixie, as far as I know, been a bug due to the use of macros in the SL code execution.  There have been compiler issues, but not bugs due to lack of type safety etc of macros.

        We may find a way to reduce the compilation time for execute - probably by using inline functions in some places - but I'm doubtful of the effect it might have on compile time.  For gcc at least, the inlining takes place prior to codegen, so that the optimizer can pass over the code and merge it into the greater body of code.  We would be pushing this in just the same way as we currently push the compiler by volume of textual code.  It is the optimization which takes time for gcc (and other compilers), not the parsing.

        Cheers

        George

         
    • Alex

      Alex - 2007-04-07

      Thanks for the extensive replies.

      I agree that perhaps for the situation you described, George, where you have to pass arbitrary functions as arguments to the macro, would be rather difficult/cumbersome to do with templates.

      But the macro's I'm talking about are generally the many that can be avoided, those that take no arguments, and seem to be macros for the sake of being macros. Take for example a look at execute.cpp, starting at line 111:

      Why do these functions need to be macros? What benefit do they have over standard, or inlined functions?

      #define clearLighting() {                                                \     currentShadingState->lightsExecuted = FALSE;                        \     *freeLights        =    *lights;                                        \     *lights            =    NULL;                                            \ }

      #define enterLightingConditional() {                                    \     int            tmpTag;                                                    \     const int    *lightTags    =    (*currentLight)->lightTags;                \     tags            =    tagStart;                                        \     for (i=numVertices;i>0;i--,tags++,lightTags++) {                    \         tmpTag = (*tags == 0);                                            \         *tags += *lightTags;                                            \         if (tmpTag && *tags) {                                            \             numActive--;                                                \             numPassive++;                                                \         }                                                                \     }                                                                    \     tags = tagStart;                                                    \ }

       
      • George Harker

        George Harker - 2007-04-08

        Hi Alex,

        These don't take arguments because they're macros.  If they were functions, then

        currentShadingState,freeLights,lights,tags,numActive,numPassive

        and probably more would all have to be parameters.

        Cheers

        George

         
    • Alex

      Alex - 2007-04-11

      What if they were private member functions in a class that appropriately encapsulates their use?

       

Log in to post a comment.