incorrect use of CCTK_BUILTIN_UNREACHABLE in Carpet

Create issue
Issue #1571 closed
Roland Haas created an issue

Carpet in commit fc35c561a049d905763de37d60690fc15473d65d "CarpetLib: Some harmless code cleanup" (sic) replaced some assert(0) by CCTK_BUILTIN_UNREACHABLE().

One of those (in the switch in transfer_p_r in line 730 of data.cc) was just found by Zach Etienne to cause segfaults if one does not properly add a new interpolation operator.

The difference is that CCTK_BUILTIN_UNREACHABLE() tells the compiler that a given line of code will never be reached thus the compiler can optimize it away. assert(0) on the other hand makes no such statement, it just aborts if it is reached. This error is compiler dependent since CCTK_BUILTIN_UNREACHABLE() expands to CCTK_Abort(0,1) unless the compiler supports __builtin_unreachable.

Keyword:

Comments (3)

  1. Erik Schnetter
    • removed comment

    One should use builtin_unreachable only in cases where code is indeed manifestly unreachable (to avoid compiler warnings), or in performance critical code (where one wouldn't want and assert either).

    You are right, this case is neither. There may be operators that prolongate in some directions, but not in others. Of course, this doesn't make sense in Carpet, but this case should be caught with an assert.

  2. Log in to comment