Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compilation failure in test_graph_node.h with disable_exceptions=false #94833

Closed
ze2j opened this issue Jul 27, 2024 · 0 comments · Fixed by #94834
Closed

Compilation failure in test_graph_node.h with disable_exceptions=false #94833

ze2j opened this issue Jul 27, 2024 · 0 comments · Fixed by #94834

Comments

@ze2j
Copy link
Contributor

ze2j commented Jul 27, 2024

Tested versions

Reproducible in 4.3 RC1 [607b230]

System information

Godot v4.3.rc (607b230) - Ubuntu 22.04.4 LTS 22.04 - X11 - Vulkan (Forward+) - dedicated NVIDIA GeForce GTX 1070 (nvidia; 535.183.01) - Intel(R) Core(TM) i7-7700K CPU @ 4.20GHz (8 Threads)

Issue description

test_graph_node.h compilation fails when disable_exception=false is used:

In file included from ./tests/test_macros.h:43,
                 from ./tests/core/config/test_project_settings.h:37,
                 from tests/test_main.cpp:40:
./tests/scene/test_graph_node.h: In lambda function:
./thirdparty/doctest/doctest.h:2355:30: error: expected primary-expression before ';' token
 2355 |         mb_name * __VA_ARGS__;                                                                     \
      |                              ^
./thirdparty/doctest/doctest.h:2345:5: note: in expansion of macro 'DOCTEST_INFO_IMPL'
 2345 |     DOCTEST_INFO_IMPL(DOCTEST_ANONYMOUS(DOCTEST_CAPTURE_),                                         \
      |     ^~~~~~~~~~~~~~~~~
./thirdparty/doctest/doctest.h:2559:77: note: in expansion of macro 'DOCTEST_INFO'
 2559 | #define DOCTEST_CHECK_NOTHROW_MESSAGE(expr, ...) DOCTEST_FUNC_SCOPE_BEGIN { DOCTEST_INFO(__VA_ARGS__); DOCTEST_CHECK_NOTHROW(expr); } DOCTEST_FUNC_SCOPE_END
      |                                                                             ^~~~~~~~~~~~
./thirdparty/doctest/doctest.h:2997:42: note: in expansion of macro 'DOCTEST_CHECK_NOTHROW_MESSAGE'
 2997 | #define CHECK_NOTHROW_MESSAGE(expr, ...) DOCTEST_CHECK_NOTHROW_MESSAGE(expr, __VA_ARGS__)
      |                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./tests/scene/test_graph_node.h:51: note: in expansion of macro 'CHECK_NOTHROW_MESSAGE'
   51 |                 CHECK_NOTHROW_MESSAGE(test_node->remove_child(test_child));
      | 

When exceptions are enabled, a compilation error occurs because CHECK_NOTHROW_MESSAGE expects an error message argument. In this case the fix would be to provide one, or use CHECK_NOTHROW instead and memdelete the child to fix the memory leak.

When exceptions are disabled the check macro (and its argument) expand to nothing: remove_child is not called and there is no memory leak because the child is still parented.

IMO the correct fix is to drop the CHECK_NOTHROW macro, always call remove_child and make sure the child is deleted. We could add a CHECK on the child count instead.

Note: CHECK_NOTHROW macro and its variant are never used in the codebase except there.

Related: Fix error message when removing only child from GraphNode #90229

Steps to reproduce

Try to build Godot's tests with disable_exceptions=false

In my case:
scons platform=linuxbsd target=editor dev_build=yes tests=true disable_exceptions=false -j9

Minimal reproduction project (MRP)

NA

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants