Provide a version of CCTK_WARN which never returns

Issue #1186 closed
Ian Hinder created an issue

CCTK_WARN(0,...) always causes a fatal abort, but the compiler does not know this. Hence, it incorrectly warns that variables might be used without being set because it doesn't know that the only path that doesn't set them terminates the program. I propose introducing a new function CCTK_WARN_ABORT(...) which calls CCTK_WARN(0,...) and is declared with the attribute noreturn (http://gcc.gnu.org/onlinedocs/gcc-3.2/gcc/Function-Attributes.html) so that the compiler knows that it can never return, and does not make incorrect conclusions when computing variable dependencies (CCTK_Abort is already defined without an error message).

Keyword:

Comments (14)

  1. Frank Löffler
    • removed comment

    This sounds like a good idea. However, is it known whether other compilers than gcc honor this attribute? If not this would only silence the warning if gcc is used. However, there could be a new macro like the proposed CCTK_ERROR which is actually calling 'exit()' after CCTK_WARN(0) (which would never be really called, but the compiler would see it). If we go that way, CCTK_VError would also me handy, analogous to CCTK_VWarn.

    If one would like to be picky then one could also point out that a negative error level is allowed in Cactus (but probably shouldn't, which is another issue). That means CCTK_WARN(0,"") does in fact not necessarily abort right now (but again, probably should).

  2. Roland Haas
    • removed comment

    Having a VError macro sounds like a good idea to me (in case not all compilers support the noreturn attribute in the same manner). C99 supports variadic macros so we should be fine in all C code. C++ hopefully also uses the same preprocessor and supports the variadic macros as an extension (they are apparently in C++ 2011 whcih so far we do not require).

  3. Frank Löffler
    • removed comment

    I don't understand your comment about VError and the noreturn attribute. The idea of that macro is to be variadic, much like CCTK_VWarn already is - to be able to compose nice error messages depending on something dynamic. It wouldn't change anything about the noreturn attribute.

    Currently CCTK_WARN a function (through a #define). CCTK_ERROR (and CCTK_VError) could be the same, but could also contain (within a block) a call to exit - something a compiler hopefully sees as 'noreturn' without even using that attribute. That call wouldn't ever be executed ...

  4. Roland Haas
    • removed comment

    Sorry for being not clear. I'll try to clarify.

    I believe right now CCTK_VWarn is the actual function and not a macro while CCTK_WARN is a macro (see src/include/cctk_WarnLevel.h and src/include/cctk_core.h). I assume that attributes can only be used with functions. So if we for some reason cannot use the noreturn attribute with all compilers we need to support, then your suggestion of having a macro with an exit() call inside of the usual

    #define CCTK_VError(...)     \
    do {                         \
      CCTK_VWarn(0,__VA_ARGS__); \
      exit(0);                   \
    } while(0)
    

    is a good solution I think. I seem to have confused CCCTK_VError and CCTK_ERROR a bit though in my original comment. I summary: I think both CCTK_ERROR and CCTK_VError would be useful (in whatever incarnation).

  5. Erik Schnetter
    • removed comment

    I would instead implement CCTK_ERROR and its variants in the same way as CCTK_WARN, except for omitting the warning level and adding a noreturn attribute. I assume most compilers (that we care about) will understand such an attribute.

  6. Frank Löffler
    • removed comment

    I would probably omit the do-while (only using a pair of {}):

    #define CCTK_VError(...)     \
    {                             \
      CCTK_VWarn(0,__VA_ARGS__); \
      exit(0);                   \
    }
    

    As to the attribute: we could set it but we otherwise often try to conform to standards most of the time, and IMHO we should if we can. I am not sure if all 'important compilers' support this attribute and before we use it we should be sure that all do. Then of course we might hit the problem that codes already using

      CCTK_WARN(0,"");
      exit(1);
    

    could produce a warning that the call to exit cannot be reached if a compiler suddenly knows that CCTK_WARN will not return. :)

  7. Erik Schnetter
    • removed comment

    We can't use { } in a macro without do...while; this would do very strange things if people called CCTK_VError in an if statement that also has an else branch.

    The noreturn attribute would, same as all other attributes, be autoconfigured. It would be automatically left out if a compiler doesn't support it.

  8. Erik Schnetter
    • changed status to open
    • removed comment

    The enclosed patch adds CCTK_Error, CCTK_VError, and CCTK_ERROR. It also adds support for autoconfigured macros CCTK_ATTRIBUTE_FORMAT, CCTK_ATTRIBUTE_NORETURN, and CCTK_BUILTIN_UNREACHABLE.

  9. Roland Haas
    • removed comment

    Patch looks fine. Please apply. It also contains patches to AHFinderDirect. The also seems ok.

  10. Log in to comment