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.

Sign up for GitHub

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

Jump to bottom

Ensure consistent return values in "impossible" situations even if assertions are compiled out #2376

Merged
merged 2 commits into from
May 12, 2022
Merged

Ensure consistent return values in "impossible" situations even if assertions are compiled out #2376

merged 2 commits into from
May 12, 2022

Conversation

heplesser
Copy link
Contributor

This PR addresses all parts of #2295 and fixes some minor issues (typos, inconsistent naming) as well. It replaces #2296.

Fixes #2295.

@heplesser
Added missing return statements after "sure to fail" assertions,
5d85811 
systematized naming of "invalid_*" constants and fixed typos.
@heplesser heplesser added S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation. I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Apr 21, 2022
@heplesser heplesser added this to PRs in progress in Kernel via automation Apr 21, 2022
@heplesser heplesser mentioned this pull request Apr 21, 2022
Closed
@StefanBruens
Copy link

AFAICS this breaks API, as the constant names are part of public headers.

@heplesser
Copy link
Contributor Author

AFAICS this breaks API, as the constant names are part of public headers.

So far, we have never promised a stable C++ API. The closest we have come to an explicit API is the set of functions defined in nest.h. We rather focus on stability of the user interface at the Python level.

Furthermore, I'd argue that the names that are changed in nest_types.h ended in _ in the past, marking them as private implementation detail.

Kernel automation moved this from PRs in progress to PRs approved Apr 25, 2022
@StefanBruens StefanBruens mentioned this pull request Apr 27, 2022
Closed
@jougs jougs merged commit 8fe7986 into nest:master May 12, 2022
Kernel automation moved this from PRs approved to Done (PRs and issues) May 12, 2022
@heplesser heplesser deleted the fix-2295-return-past-assert branch October 4, 2022 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jougs jougs jougs approved these changes

@hakonsbm hakonsbm hakonsbm approved these changes

Assignees
No one assigned
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation.
Projects
Kernel
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

proxynode::get_thread should return defined value (or throw)
4 participants
@heplesser @StefanBruens @jougs @hakonsbm