• State Not Answered
  • Locked Locked
  • Replies 13 replies
  • Subscribers 22 subscribers
  • Views 2900 views
  • Users 0 members are here
Options
Related
How was your experience today?
This discussion has been locked.
You can no longer post new replies to this discussion. If you have a question you can start a new discussion

Function too complex

Mike Hwang

I have a function,but the compiler terminated because the function is too complex.

C51 FATAL-ERROR -
  ACTION:  GENERATING INTERMEDIATE CODE
  ERROR:   FUNCTION '_siParseDescriptor' (LINE 1058, T=400,L=132,l=1): TOO COMPLEX
C51 TERMINATED.

Parents
  • 0 in reply to Andy Neil

    I suspect the problem is just the complexity of that huge, huge, expression. When you introduce the intermediate variables, you simply the amount of code in one chunk, which I imagine is why the code starts to compile.

    What's the purpose of the "do {} while(0)" in your create macro? Simple curly braces should work as well to introduce a local block.

    Also, just as a readability and style nitpick, complicated macros are probably better defined using the backslash to allow them to occupy multiple lines.

    #define CreateSatelliteDeliverySystemDescriptor(descr, freq, orb, mod, polar, sr, fec)     {                                                                                      xCreateNode (((struct SatelliteDeliverySystemDescriptor *)descr), NULL);                                      ((struct SatelliteDeliverySystemDescriptor *)descr)->Tag = DESCR_SAT_DEL_SYS;                   ((struct SatelliteDeliverySystemDescriptor *)descr)->Frequency = freq;                          ((struct SatelliteDeliverySystemDescriptor *)descr)->OrbitalPosition = orb;                     ((struct SatelliteDeliverySystemDescriptor *)descr)->Modulation = mod;                          ((struct SatelliteDeliverySystemDescriptor *)descr)->Polarization = polar;                      ((struct SatelliteDeliverySystemDescriptor *)descr)->SymbolRate = sr;                           ((struct SatelliteDeliverySystemDescriptor *)descr)->FEC = fec;                                 }

    Get rid of the cast (or at least typedef a shorter name) and it'd be readable.

    I'd recommend against the casts in this case for safety reasons, too. Macros can't type-check their arguments, unlike functions. With the casts in place, if someone passes in a descr that's not really a SDSDesc*, the casts will just hammer away the type mismatch warning, and you'll have an ugly problem to debug. Without the casts, the compiler will generate a warning on the attempted dereference of the pointer. If a cast is necessary, let the caller supply it rather than the macro.

Reply
  • 0 in reply to Andy Neil

    I suspect the problem is just the complexity of that huge, huge, expression. When you introduce the intermediate variables, you simply the amount of code in one chunk, which I imagine is why the code starts to compile.

    What's the purpose of the "do {} while(0)" in your create macro? Simple curly braces should work as well to introduce a local block.

    Also, just as a readability and style nitpick, complicated macros are probably better defined using the backslash to allow them to occupy multiple lines.

    #define CreateSatelliteDeliverySystemDescriptor(descr, freq, orb, mod, polar, sr, fec)     {                                                                                      xCreateNode (((struct SatelliteDeliverySystemDescriptor *)descr), NULL);                                      ((struct SatelliteDeliverySystemDescriptor *)descr)->Tag = DESCR_SAT_DEL_SYS;                   ((struct SatelliteDeliverySystemDescriptor *)descr)->Frequency = freq;                          ((struct SatelliteDeliverySystemDescriptor *)descr)->OrbitalPosition = orb;                     ((struct SatelliteDeliverySystemDescriptor *)descr)->Modulation = mod;                          ((struct SatelliteDeliverySystemDescriptor *)descr)->Polarization = polar;                      ((struct SatelliteDeliverySystemDescriptor *)descr)->SymbolRate = sr;                           ((struct SatelliteDeliverySystemDescriptor *)descr)->FEC = fec;                                 }

    Get rid of the cast (or at least typedef a shorter name) and it'd be readable.

    I'd recommend against the casts in this case for safety reasons, too. Macros can't type-check their arguments, unlike functions. With the casts in place, if someone passes in a descr that's not really a SDSDesc*, the casts will just hammer away the type mismatch warning, and you'll have an ugly problem to debug. Without the casts, the compiler will generate a warning on the attempted dereference of the pointer. If a cast is necessary, let the caller supply it rather than the macro.

Children
  • 0 in reply to Drew Davis

    too comples to compile
    how about "too complex to rtead"

    Why does "real C" programmers insist on cramming more than a human can understand into a single statement.

    As far as I can see this can easiy be broken up in 4-10 understandable statements.

    Is there really a reason to make it as tough as posssible on the next guy/gal assigned maintenance of the code?

    Erik

  • 0 in reply to Drew Davis

    "What's the purpose of the "do {} while(0)" in your create macro?
    Simple curly braces should work as well to introduce a local block."

    Not quite. Consider the use of a function-like macro (i.e., terminated with a semicolon)
    in an if/else clause expanding to the curly brace from only. Please refer to:

    http://www.eskimo.com/~scs/C-faq/q10.4.html

    The do/while(0) method is the safe way to define a function-like (multi-statement) macro.