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

Ensure more functions do not panic #265

Merged
merged 1 commit into from
Aug 1, 2022
Merged

Conversation

ankane
Copy link
Contributor

@ankane ankane commented Jul 30, 2022

This PR:

  • adds #[cfg_attr(all(test, assert_no_panic), no_panic::no_panic)] to more functions
  • removes panics from exp10 and exp10f

This leaves frexp and frexpf as the only remaining functions that can panic (wasn't able to figure out how to fix).

Edit: it looks like j1f (and possibly j1 can also panic)

Edit 2: Not sure why, but it looks like the tests didn't catch integer division in lgamma_r/lgammaf_r

@Amanieu
Copy link
Member

Amanieu commented Jul 31, 2022

j1f and j1 seem to have indexing which might be calling panics.

@ankane
Copy link
Contributor Author

ankane commented Jul 31, 2022

Quick update: From what I can tell, the panics for frexp/frexpf and j1/j1f are due to recursion. The tests pass for j1 but not j1f since j1f has this additional test. Other functions with recursion like j0 and j0f also likely can panic, but the tests don't catch it.

Edit: I've removed all the j* functions for this PR.

@Amanieu Amanieu merged commit c108db9 into rust-lang:master Aug 1, 2022
@Amanieu
Copy link
Member

Amanieu commented Aug 1, 2022

Thanks!

@ankane
Copy link
Contributor Author

ankane commented Aug 1, 2022

Thanks @Amanieu!

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