wlav / cppyy

Other
384 stars 38 forks source link

Should assert() work? #235

Open Alex-EEE opened 1 month ago

Alex-EEE commented 1 month ago

Question, should assert work out of the box with cppyy? I'm also running with export EXTRA_CLING_ARGS='-Wno-asm-operand-widths', so that means -DNDEBUG shouldn't be getting passed in correct?

wlav commented 1 month ago

What does "work out of the box" mean? assert() is a macro, not a function that can be bound/called from Python.

For the C++ side, you can check with cppyy.cppexec("std::cerr << NDEBUG << std::endl;") whether NDEBUG is defined. There is very limited support for some macros. In this case, you could also check for cppyy.macro('NDEBUG').

Alex-EEE commented 1 month ago

Sorry for the confusion, what I mean is, have you seen assert cause a function running with cppyy to stop, throw an error, or otherwise get the attention of the user (ie, what assert is used for)

As is, I have a piece of code that I just realized should have been triggering an assert (or failing an assert), so I was just wondering if there's anyway to make assert work. I was thinking that maybe NDEBUG was sneaking in and that's why the assert didn't run.

Thanks for that snippet, I'll use that to check it NDEBUG is being set somewhere

wlav commented 1 month ago

Found it ... it's hard-wired in cling's CIFactory.cpp:

    // cling wants to JIT O1 by default. Might want to revisit once we have
    // debug symbols.
    PPOpts.addMacroDef("NDEBUG=1");

I can ask upstream tomorrow whether that's really necessary as the comment seems to suggest.

wlav commented 1 month ago

Per upstream, Clang's behavior is to enable NDEBUG if optimization level is greater than 0. Pre-compiled headers/modules should not be mixed with interpreter code where NDEBUG differs, as there could be ABI differences as a consequence (e.g. extra data members to a class for tracking debug info).

Still doesn't mean that it hast to be hard-wired: they could have tied it to the chosen optimization level, so that you could switch if off by setting -O0, but the above is their reasoning.