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

Do not put -DNDEBUG into hpx_application.pc #1160

Closed
eschnett opened this Issue Jun 20, 2014 · 3 comments

Comments

Projects
None yet
3 participants
@eschnett
Contributor

eschnett commented Jun 20, 2014

HPX should not put -DNDEBUG into its hpx_application.pc. The choice of using -DNDEBUG or not should belong to the application, not a library (HPX) that an application uses. It is often very convenient to be able to use assert() to track down bugs, even in production code. -DNDEBUG prevents this, forcing people to roll their own assert statement.

Since HPX already uses HPX_ASSERT, why does it care about -DNDEBUG? This seems like an overzealous optimization to me; HPX_DISABLE_ASSERTS should suffice.

@hkaiser

This comment has been minimized.

Show comment
Hide comment
@hkaiser

hkaiser Jun 20, 2014

Member

NDEBUG can be safely removed there, we don't need it anymore. Thanks for catching this!

Member

hkaiser commented Jun 20, 2014

NDEBUG can be safely removed there, we don't need it anymore. Thanks for catching this!

@hkaiser hkaiser added this to the 0.9.9 milestone Jun 20, 2014

@sithhell sithhell self-assigned this Jul 11, 2014

@sithhell

This comment has been minimized.

Show comment
Hide comment
@sithhell

sithhell Jul 11, 2014

Member

The c stdlib headers actually need it (plain assert for example). But i agree with Erik that it should be removed from the compile flags. I am on it

Member

sithhell commented Jul 11, 2014

The c stdlib headers actually need it (plain assert for example). But i agree with Erik that it should be removed from the compile flags. I am on it

@sithhell

This comment has been minimized.

Show comment
Hide comment
@sithhell

sithhell Sep 3, 2014

Member

This should be fixed by ecdb034. Please reopen if the problem still persists.

Member

sithhell commented Sep 3, 2014

This should be fixed by ecdb034. Please reopen if the problem still persists.

@sithhell sithhell closed this Sep 3, 2014

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