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

Merge MANIFOLD_EXCEPTIONS and MANIFOLD_DEBUG #1041

Open
elalish opened this issue Nov 14, 2024 · 1 comment · May be fixed by #1044
Open

Merge MANIFOLD_EXCEPTIONS and MANIFOLD_DEBUG #1041

elalish opened this issue Nov 14, 2024 · 1 comment · May be fixed by #1044
Milestone

Comments

@elalish
Copy link
Owner

elalish commented Nov 14, 2024

Currently our readme says our MANIFOLD_DEBUG flag controls both assertions and exceptions, but in fact we have a second CMake flag: MANIFOLD_EXCEPTIONS, which defaults to ON. However, nearly all of our exceptions use DEBUG_ASSERT, for which both debug and exceptions must be enabled to fire. Only Vec uses ASSERT (and hull, but I think that's a mistake) which is enabled even when debug is off. I can't imagine a situation where an app is going to be able to do much if we throw a Vec assertion - it's basically just a seg-fault, and it certainly shouldn't happen if we've written our code properly.

I propose we merge MANIFOLD_EXCEPTIONS into MANIFOLD_DEBUG and use only a single kind of ASSERT. This will have the effect of turning exceptions off by default, which seems right? Feedback welcome - and WDYT @pca006132?

@elalish elalish added this to the v3.0 milestone Nov 14, 2024
@pca006132
Copy link
Collaborator

Sounds good to me.

@pca006132 pca006132 changed the title Disable exceptions by default Merge MANIFOLD_EXCEPTIONS and MANIFOLD_DEBUG Nov 14, 2024
@elalish elalish linked a pull request Nov 14, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants