• State Not Answered
  • Locked Locked
  • Replies 13 replies
  • Subscribers 21 subscribers
  • Views 2844 views
  • Users 0 members are here
2025 survey

We are running a survey to help us improve the experience for all of our members. If you see the survey appear, please take the time to tell us about your experience if you can.

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.