Fix compilation for Clang 19

merlinND commented 4 months ago

Description

The bindings cause compilation error with Clang 19. Outside of a templated context, the code in a false if constexpr can still be instantiated. With Clang 19, this led to instantiate e.g. dr::bind_array_t<const Object *>, which is invalid.

~~This PR requires a DrJit-side helper to be added: https://github.com/mitsuba-renderer/drjit/pull/234~~ Edit: DrJit-side helper no longer needed.

This PR also updates the embree submodule to also include a fix for Clang 19.

Testing

No local testing, letting the CI check other compilers.

Checklist

[x] My code follows the style guidelines of this project
[x] My changes generate no new warnings (on Clang 19)
[x] My code also compiles for cuda_* and llvm_* variants. If you can't test this, please leave below
[x] I have commented my code
[ ] I have made corresponding changes to the documentation
[ ] I have added tests that prove my fix is effective or that my feature works
[x] I cleaned the commit history and removed any "Merge" commits
[x] I give permission that the Mitsuba 3 project may redistribute my contributions under the terms of its license

merlinND commented 4 months ago

Converting to draft PR because there are still compilation errors to fix.

merlinND commented 4 months ago

Edit: actually the Mitsuba build works for me if I base https://github.com/mitsuba-renderer/drjit/pull/234 off of the current DrJit pointer (https://github.com/mitsuba-renderer/drjit/tree/42b27b4343b4ae7c1a4df75ba54f4ab7bc7b3fd1), so it's some of the other recent DrJit changes that Mitsuba is not compatible with yet.

Edit 2: I've updated the DrJit PR to target the mitsuba branch of DrJit.

merlinND commented 4 months ago

I can reproduce the segfaults from the CI locally, even on master, compiling with Clang 15. About 50% of the time, I get a segfault during the cleanup phase of Pytest:

$ python -m pytest -v '../src/integrators/tests/test_ptracer.py::test01_render_simple' 
(...)
Thread 1 "python" received signal SIGSEGV, Segmentation fault.
0x000000000061cd10 in ?? ()
(gdb) bt
#0  0x000000000061cd10 in ?? ()
#1  0x000000000061c9cc in ?? ()
#2  0x00000000006bce48 in PyGC_Collect ()
#3  0x00000000006b0e8c in Py_FinalizeEx ()
#4  0x00000000006bc891 in Py_RunMain ()
#5  0x00000000006bc4ad in Py_BytesMain ()
#6  0x00007ffff7c2a1ca in __libc_start_call_main (main=main@entry=0x518a50, argc=argc@entry=5, argv=argv@entry=0x7fffffffd6f8) at ../sysdeps/nptl/libc_start_call_main.h:58
#7  0x00007ffff7c2a28b in __libc_start_main_impl (main=0x518a50, argc=5, argv=0x7fffffffd6f8, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffd6e8) at ../csu/libc-start.c:360
#8  0x0000000000657925 in _start ()

njroussel commented 4 months ago

Hi @merlinND

We're looking into the segfault during shutdown.

As for the original content of this PR, it looks good - I'm a bit suprised we've been getting away with the current code :sweat_smile:. Have you considered arbitraily templating the MI_PY_EXPORT functions wherever needed? That would also fix this issue, and I don't think it comes at a particular compilation cost as these files are compiled exactly once per variant anyway (due to the MI_VARIANT_FLOAT macro). It just seems like a less intrusive fix, or am I forgetting something?

merlinND commented 4 months ago

Very good point @njroussel! I force-pushed a different implementation which locally forwards the python_export_... to a templated function in the macro, which solves the problem everywhere rather than having to introduce a different bind_if_array_t call manually each time.

njroussel commented 4 months ago

Great, if that works can I also ask you to remove the bind_if_array_t change in Dr.Jit and then I'll merge this.

(No point in blocking this due to the leak/segfault produced by earlier commits)

merlinND commented 4 months ago

Done! Poor CI is working hard today :smile:

mitsuba-renderer / mitsuba3