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

Fix for #750 #751

Merged
merged 1 commit into from
Jan 29, 2022
Merged

Fix for #750 #751

merged 1 commit into from
Jan 29, 2022

Conversation

mborland
Copy link
Member

No description provided.

@mborland mborland linked an issue Jan 28, 2022 that may be closed by this pull request
@mborland
Copy link
Member Author

mborland commented Jan 29, 2022

Once boostorg/multiprecision#422 is merged I will merge this and we should be back to green.

@NAThompson I don't see anything in quartic_roots that stands out as not working on MSVC.

@ckormanyos
Copy link
Member

Once (boostorg/multiprecision#422)[https://github.com/boostorg/multiprecision/pull/422] is merged ...

Done. Thanks everyone for driving forward so dedicatedly on this.

@mborland mborland merged commit 2cc734d into boostorg:develop Jan 29, 2022
@mborland mborland deleted the 750 branch January 29, 2022 11:58
@NAThompson
Copy link
Collaborator

@mborland : Sorry, do I need to do anything? I'm confused about what in the quartic roots code caused the issue. . . .

@mborland
Copy link
Member Author

@NAThompson I don't think anything needs to happen. I couldn't find any compelling reason that quartic roots would cause the issue either. I would wait and see if anybody reports and issue from the field.

@jzmaddock
Copy link
Collaborator

Sorry, do I need to do anything? I'm confused about what in the quartic roots code caused the issue

Unfortunately there is an issue (though I admit the msvc std lib is being a bit anal about this, if technically correct).

The problem is that from latest msvc-14.2 onwards, the std lib will assert if you attempt to sort anything that contains NaN's. This is technically correct in that the behaviour is undefined in this case, we just haven't hit a std library which checked that before. The problem here is that the quartic roots code, will attempt to sort a list containing Nan's in almost every case.... so yes, the code is now completely broken with latest msvc. I would expect -fsanitize=undefined to trap this as well, but I haven't checked that. IMO we need to be just sorting the N roots found, and not the NaN's at the end of the list.

Hope that all makes sense.

@NAThompson
Copy link
Collaborator

@jzmaddock : Welp . . . yup this is a bug users will hit then. Will attempt a fix.

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.

CI Hangs
4 participants