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

<xiosbase>: Remove non-conforming extension ios_base::hexfloat #4345

Merged

Conversation

frederick-vs-ja
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja commented Jan 27, 2024

Fixes #3296.

All uses of ios_base::hexfloat are replaced with ios_base::scientific | ios_base::fixed. Also, it seems to me that we should no longer test ios_base::hexfloat.

If there's too many breaking for removing, we may fallback to deprecation.

@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner January 27, 2024 18:38
@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Jan 28, 2024
@StephanTLavavej
Copy link
Member

I previously thought that we couldn't remove ios_base::hexfloat until vNext, but in #4125 shipping in VS 2022 17.9 Preview 2 I was able to remove other static constexpr data members. (The enums had to be retained due to an internal tool that we didn't have time to fix.)

I think that TR1-era ios_base::hexfloat was sufficiently obscure that we should try removing this outright, given how simple it is to fix any affected code (as you mentioned, (std::ios_base::scientific | std::ios_base::fixed) is a targeted replacement, std::hexfloat is only slightly more work). Worst case, a bunch of Real World Code breaks and we have to reluctantly go back to your deprecation approach.

@frederick-vs-ja
Copy link
Contributor Author

Worst case, a bunch of Real World Code breaks and we have to reluctantly go back to your deprecation approach.

Hmm, it seems that there're some codes on Github relying on ios::hexfloat.

@StephanTLavavej
Copy link
Member

I'm willing to break a little code at compile-time, when that code is non-portable and easily fixed. Many changes (including even deprecation warnings!) can result in compiler errors, it's just a question of their magnitude and how easy they are to deal with.

That said, it would be an option to deprecate in 17.10, and wait until the next update for outright removal. I just don't think we need to move that slowly.

@frederick-vs-ja frederick-vs-ja changed the title <xiosbase>: Deprecate non-conforming extension ios_base::hexfloat <xiosbase>: Remove non-conforming extension ios_base::hexfloat Jan 28, 2024
@frederick-vs-ja
Copy link
Contributor Author

Thanks! I've pushed a commit for removal.

@StephanTLavavej StephanTLavavej self-assigned this Jan 28, 2024
@StephanTLavavej StephanTLavavej removed their assignment Jan 30, 2024
@StephanTLavavej StephanTLavavej self-assigned this Jan 30, 2024
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit c5e9583 into microsoft:main Jan 30, 2024
35 checks passed
@StephanTLavavej
Copy link
Member

Thanks for removing this hex from the Standard Library! 🪄 😸 🎉

@StephanTLavavej StephanTLavavej added bug Something isn't working and removed enhancement Something can be improved labels Jan 30, 2024
@frederick-vs-ja frederick-vs-ja deleted the deprecate-ios_base-hexfloat branch January 30, 2024 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

<xiosbase>: Non-conforming extension ios_base::hexfloat
2 participants