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

Switch asserts to runtime errors in constructors #345

Open
jhp-lanl opened this issue Feb 14, 2024 · 3 comments
Open

Switch asserts to runtime errors in constructors #345

jhp-lanl opened this issue Feb 14, 2024 · 3 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request Robustness Ensures that existing features work as intended

Comments

@jhp-lanl
Copy link
Collaborator

Our spack host codes don't always propagate the build type to their spack dependencies. As a result, telling the host code to build with debug won't link to a debug version of singularity-eos. Host codes also don't want to propagate the build type either for performance reasons or to minimize the size of the spack buildout.

However, this makes our assert statements almost completely useless since it's probably rare that somebody will manually change the spack spec to build singularity in debug mode.

I would propose that all constructors should use the ports-of-call portable error machinery instead and ensure that errors are raised even outside debug builds. Checks at lookup time can still use asserts since we want those to be performant, but there's no reason to sacrifice debugging for performance during initialization.

@jhp-lanl jhp-lanl added bug Something isn't working enhancement New feature or request Robustness Ensures that existing features work as intended labels Feb 14, 2024
@jhp-lanl jhp-lanl self-assigned this Feb 14, 2024
@Yurlungur
Copy link
Collaborator

We have a tool for this now in the portable_errors header. PORTABLE_ALWAYS_THROW seems like the thing.

@jhp-lanl
Copy link
Collaborator Author

We have a tool for this now in the portable_errors header. PORTABLE_ALWAYS_THROW seems like the thing.

I would propose that all constructors should use the ports-of-call portable error machinery instead and ensure that errors are raised even outside debug builds

😉

@Yurlungur
Copy link
Collaborator

Whoops clearly I have excellent reading comprehension. My bad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request Robustness Ensures that existing features work as intended
Projects
None yet
Development

No branches or pull requests

2 participants