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

Code cleanups: Unify _Float_traits and _Floating_type_traits #1442

Merged
merged 6 commits into from
Dec 1, 2020

Conversation

StephanTLavavej
Copy link
Member

@StephanTLavavej StephanTLavavej commented Nov 11, 2020

  • tr1: _HAS_WINDOWS_FILESYSTEM is unused.
    • We don't usually clean up tr1, but this was polluting my search for #ifdef _HAS.
  • cond.cpp, mutex.cpp: Use alignof.
  • Hyphenate floating-point in comments.
  • Fix "non meow" comments (usually to say "non-meow", sometimes rephrasing differently).
  • Replace _Float_traits usage with _Floating_type_traits from <type_traits>.
    • While @statementreply was working on <complex>: Improve numerical accuracy of sqrt and log #935 which added _Float_traits to <xutility>, Finish P0768R1 by adding Spaceship CPOs #1370 moved _Floating_type_traits up from <charconv> to <type_traits>. We should de-duplicate this machinery now. _Floating_type_traits has all of this content and more (under different names), except for _Float_traits::_Magnitude_mask. This is used only once in product code (_Float_abs_bits), where I believe it's simpler to refer to ~_Floating_type_traits::_Shifted_sign_mask (as we're clearly masking away the sign bit), instead of adding _Magnitude_mask to _Floating_type_traits.
    • I verified with static_assert that the values are identical.
    • Note that there is a terminology difference: _Floating_type_traits has _Shifted_exponent_mask (which we want; it's been "shifted" into the proper position for single-precision or double-precision) and _Exponent_mask (which we do not want). I double-checked that all uses of _Float_traits::_Exponent_mask were properly converted to _Floating_type_traits::_Shifted_exponent_mask.
    • This extracts _Traits and _Uint_type typedefs for readability.
    • This also updates the test code's definition of magnitude_mask_v; the constants need to be re-ordered which is why the diff looks complicated.

@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Nov 11, 2020
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner November 11, 2020 01:26
stl/inc/complex Outdated Show resolved Hide resolved
stl/src/cond.cpp Outdated Show resolved Hide resolved
stl/src/mutex.cpp Outdated Show resolved Hide resolved
@mnatsuhara mnatsuhara self-assigned this Nov 11, 2020
@StephanTLavavej StephanTLavavej self-assigned this Dec 1, 2020
@StephanTLavavej StephanTLavavej merged commit d06561f into microsoft:master Dec 1, 2020
@StephanTLavavej StephanTLavavej deleted the cleanups branch December 1, 2020 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants