Skip to content

Nef_S2: Skip running assertion code when runtime assertions are disabled#7212

Closed
GilesBathgate wants to merge 1 commit intoCGAL:mainfrom
GilesBathgate:Nef_3-skip_assertion_code_SM_const_decorator-GilesBathgate
Closed

Nef_S2: Skip running assertion code when runtime assertions are disabled#7212
GilesBathgate wants to merge 1 commit intoCGAL:mainfrom
GilesBathgate:Nef_3-skip_assertion_code_SM_const_decorator-GilesBathgate

Conversation

@GilesBathgate
Copy link
Copy Markdown
Contributor

Summary of Changes

Since CGAL_assertion_code ignores the state of CGAL::get_use_assertions() some assertion code in SM_const_decorator.h gets unnecessarily run. The only sensible thing to me seems to be to add some additional CGAL_assertion_code to skip everything when runtime assertions are disabled.

Release Management

  • Affected package(s): Nef_3/Nef_S2
  • Issue(s) solved (if any): performance/bugfix
  • License and copyright ownership: CGAL authors

@GilesBathgate
Copy link
Copy Markdown
Contributor Author

@afabri I also request for your comment regarding: 9b357c6#r97801479

@afabri
Copy link
Copy Markdown
Member

afabri commented Jan 26, 2023

In case it only concerns check_integrity_and_topological_planarity(bool faces) would it not make sense to put that one in a CGAL_assertion(..)` and to replace the assertions in the function itself?

@GilesBathgate
Copy link
Copy Markdown
Contributor Author

would it not make sense to put that one in a CGAL_assertion(..)` and to replace the assertions in the function itself?

That would be an appropriate alternative. I also did it this way for brevity, and to keep the code active when CGAL_NEF_TRACEN/CGAL_USE_TRACE is active.

@afabri
Copy link
Copy Markdown
Member

afabri commented Jan 31, 2023

This PR is also the right moment to fix this warning concerning this line (as well as line 310) where we have to put CGAL_assertion_code() around.

@GilesBathgate GilesBathgate force-pushed the Nef_3-skip_assertion_code_SM_const_decorator-GilesBathgate branch from 1df07b2 to d1ee393 Compare February 26, 2023 12:29
@GilesBathgate
Copy link
Copy Markdown
Contributor Author

This PR is also the right moment to fix this warning concerning this line (as well as line 310) where we have to put CGAL_assertion_code() around.

Fixed in #7260

@sloriot sloriot changed the base branch from master to main September 18, 2025 05:58
@GilesBathgate GilesBathgate marked this pull request as draft April 1, 2026 20:12
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 this pull request may close these issues.

2 participants