Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@StephanTLavavej
Copy link
Member

Followup to #5836.

I believe that there are no functional changes here. I've verified that the dllexport surface is unaffected.

Commits

  • Fuse xfcosh.cpp into xcosh.cpp.
  • Fuse xfsinh.cpp into xsinh.cpp.
  • Fuse xfdtest.cpp into xdtest.cpp.
  • Fuse xfexp.cpp into xexp.cpp.
  • Fuse xfdscale.cpp into xexp.cpp.
    • _FDscale() wasn't dllexported, so we can move it into an unnamed namespace.
  • Fuse xfdnorm.cpp into xexp.cpp.
    • _FDnorm() wasn't dllexported, so we can move it into an unnamed namespace.
  • Replace _Feraise() with errno = ERANGE;.
    • It had no other effect. I'm adding // underflow versus // overflow comments to preserve clarity. _Feraise() wasn't dllexported. It was marked __CLRCALL_PURE_OR_CDECL but not _CRTIMP2_PURE. Remove now-unused macros.
  • Fuse underflow/overflow cases, avoid missing break;.
  • Preserve _Dtest() for bincompat, replaced with fpclassify().
    • They return the same values, see below.
  • _FINITE => FP_NORMAL
  • _INFCODE => FP_INFINITE
  • _NANCODE => FP_NAN
  • 0 => FP_ZERO, carefully.
  • Remove now-unused _Dtest() macros.
  • Replace DSIGN and FSIGN with signbit().
    • See below for an equivalence proof.
  • Move _Dval, _Fval, and their index macros into xexp.cpp's unnamed namespace.
    • It's okay to move these unions out of _EXTERN_C_UNLESS_PURE, because nothing else uses them.
  • Change index macros to constexpr int, cleanup comments.
  • Move _Xfe_overflow(), _Xfe_underflow() into xexp.cpp's unnamed namespace.
    • They obviously aren't dllexported.
  • Replace extern const _Xbig and _FXbig with replicated constexpr.
    • They weren't dllexported. Although this results in a bit of code duplication, the result is more optimizer-friendly, and will allow us to eliminate xmath.hpp.
  • Delete xmath.hpp, include only relevant headers.
  • Fix dangerous _Dval, _Fval union sizes.
    • These are within an unnamed namespace, so we can alter their sizes. There was no actual harm here, since we never indexed into the nonexistent memory, but it was still risky, and likely to annoy static analysis someday.

Equivalence Proofs

_Dtest is fpclassify

C:\Temp>type meow.cpp
#include <cmath>
#include <limits>
#include <print>
using namespace std;

static_assert(FP_SUBNORMAL == -2); // _DENORM
static_assert(FP_NORMAL == -1); // _FINITE
static_assert(FP_ZERO == 0);
static_assert(FP_INFINITE == 1); // _INFCODE
static_assert(FP_NAN == 2); // _NANCODE

extern "C" _CRTIMP2_PURE short __CLRCALL_PURE_OR_CDECL _Dtest(double*) noexcept;

void classify(double dbl) {
    const short stl = _Dtest(&dbl);
    const int crt   = fpclassify(dbl);
    println("dbl: {:6}; stl: {:2}; crt: {:2}; Equal: {}", dbl, stl, crt, stl == crt);
}

int main() {
    classify(numeric_limits<double>::denorm_min());
    classify(3.14);
    classify(0.0);
    classify(-0.0);
    classify(numeric_limits<double>::infinity());
    classify(numeric_limits<double>::quiet_NaN());
}
C:\Temp>cl /EHsc /nologo /W4 /std:c++latest /MTd /Od meow.cpp
meow.cpp

C:\Temp>meow
dbl: 5e-324; stl: -2; crt: -2; Equal: true
dbl:   3.14; stl: -1; crt: -1; Equal: true
dbl:      0; stl:  0; crt:  0; Equal: true
dbl:     -0; stl:  0; crt:  0; Equal: true
dbl:    inf; stl:  1; crt:  1; Equal: true
dbl:    nan; stl:  2; crt:  2; Equal: true

DSIGN is signbit

C:\Temp>type meow.cpp
#include <cmath>
#include <limits>
#include <print>
using namespace std;

union _Dval {
    unsigned short _Sh[8];
    double _Val;
};
union _Fval {
    unsigned short _Sh[8];
    float _Val;
};

#define _D0 3
#define _F0 1

#define DSIGN(x) (reinterpret_cast<_Dval*>(&(x))->_Sh[_D0] & _DSIGN)
#define FSIGN(x) (reinterpret_cast<_Fval*>(&(x))->_Sh[_F0] & _FSIGN)

void test_dbl(double dbl) {
    const bool macro = static_cast<bool>(DSIGN(dbl));
    println("dbl: {:4}; macro: {:5}; signbit: {:5}; Equal: {}", dbl, macro, signbit(dbl), macro == signbit(dbl));
}

void test_flt(float flt) {
    const bool macro = static_cast<bool>(FSIGN(flt));
    println("flt: {:4}; macro: {:5}; signbit: {:5}; Equal: {}", flt, macro, signbit(flt), macro == signbit(flt));
}

int main() {
    test_dbl(numeric_limits<double>::infinity());
    test_dbl(-numeric_limits<double>::infinity());
    test_flt(numeric_limits<float>::infinity());
    test_flt(-numeric_limits<float>::infinity());
}
C:\Temp>cl /EHsc /nologo /W4 /std:c++latest /MTd /Od meow.cpp
meow.cpp

C:\Temp>meow
dbl:  inf; macro: false; signbit: false; Equal: true
dbl: -inf; macro: true ; signbit: true ; Equal: true
flt:  inf; macro: false; signbit: false; Equal: true
flt: -inf; macro: true ; signbit: true ; Equal: true

`_FDscale()` wasn't dllexported, so we can move it into an unnamed namespace.
`_FDnorm()` wasn't dllexported, so we can move it into an unnamed namespace.
It had no other effect. I'm adding `// underflow` versus `// overflow` comments to preserve clarity.

`_Feraise()` wasn't dllexported. It was marked `__CLRCALL_PURE_OR_CDECL` but not `_CRTIMP2_PURE`.

Remove now-unused macros.
They return the same values:

    C:\Temp>type meow.cpp
    #include <cmath>
    #include <limits>
    #include <print>
    using namespace std;

    static_assert(FP_SUBNORMAL == -2); // _DENORM
    static_assert(FP_NORMAL == -1); // _FINITE
    static_assert(FP_ZERO == 0);
    static_assert(FP_INFINITE == 1); // _INFCODE
    static_assert(FP_NAN == 2); // _NANCODE

    extern "C" _CRTIMP2_PURE short __CLRCALL_PURE_OR_CDECL _Dtest(double*) noexcept;

    void classify(double dbl) {
        const short stl = _Dtest(&dbl);
        const int crt   = fpclassify(dbl);
        println("dbl: {:6}; stl: {:2}; crt: {:2}; Equal: {}", dbl, stl, crt, stl == crt);
    }

    int main() {
        classify(numeric_limits<double>::denorm_min());
        classify(3.14);
        classify(0.0);
        classify(-0.0);
        classify(numeric_limits<double>::infinity());
        classify(numeric_limits<double>::quiet_NaN());
    }

    C:\Temp>cl /EHsc /nologo /W4 /std:c++latest /MTd /Od meow.cpp
    meow.cpp

    C:\Temp>meow
    dbl: 5e-324; stl: -2; crt: -2; Equal: true
    dbl:   3.14; stl: -1; crt: -1; Equal: true
    dbl:      0; stl:  0; crt:  0; Equal: true
    dbl:     -0; stl:  0; crt:  0; Equal: true
    dbl:    inf; stl:  1; crt:  1; Equal: true
    dbl:    nan; stl:  2; crt:  2; Equal: true
Equivalence proof:

    C:\Temp>type meow.cpp
    #include <cmath>
    #include <limits>
    #include <print>
    using namespace std;

    union _Dval {
        unsigned short _Sh[8];
        double _Val;
    };
    union _Fval {
        unsigned short _Sh[8];
        float _Val;
    };

    #define _D0 3
    #define _F0 1

    #define DSIGN(x) (reinterpret_cast<_Dval*>(&(x))->_Sh[_D0] & _DSIGN)
    #define FSIGN(x) (reinterpret_cast<_Fval*>(&(x))->_Sh[_F0] & _FSIGN)

    void test_dbl(double dbl) {
        const bool macro = static_cast<bool>(DSIGN(dbl));
        println("dbl: {:4}; macro: {:5}; signbit: {:5}; Equal: {}", dbl, macro, signbit(dbl), macro == signbit(dbl));
    }

    void test_flt(float flt) {
        const bool macro = static_cast<bool>(FSIGN(flt));
        println("flt: {:4}; macro: {:5}; signbit: {:5}; Equal: {}", flt, macro, signbit(flt), macro == signbit(flt));
    }

    int main() {
        test_dbl(numeric_limits<double>::infinity());
        test_dbl(-numeric_limits<double>::infinity());
        test_flt(numeric_limits<float>::infinity());
        test_flt(-numeric_limits<float>::infinity());
    }

    C:\Temp>cl /EHsc /nologo /W4 /std:c++latest /MTd /Od meow.cpp
    meow.cpp

    C:\Temp>meow
    dbl:  inf; macro: false; signbit: false; Equal: true
    dbl: -inf; macro: true ; signbit: true ; Equal: true
    flt:  inf; macro: false; signbit: false; Equal: true
    flt: -inf; macro: true ; signbit: true ; Equal: true
… namespace.

It's okay to move these unions out of _EXTERN_C_UNLESS_PURE,
because nothing else uses them.
…mespace.

They obviously aren't dllexported.
They weren't dllexported.

Although this results in a bit of code duplication, the result is more optimizer-friendly, and will allow us to eliminate xmath.hpp.
These are within an unnamed namespace, so we can alter their sizes.

There was no actual harm here, since we never indexed into the nonexistent memory,
but it was still risky, and likely to annoy static analysis someday.
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner December 12, 2025 13:30
@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Dec 12, 2025
@github-project-automation github-project-automation bot moved this to Initial Review in STL Code Reviews Dec 12, 2025
stl/src/xvalues.cpp Show resolved Hide resolved
@StephanTLavavej StephanTLavavej moved this from Initial Review to Final Review in STL Code Reviews Dec 12, 2025
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

Status: Final Review

Development

Successfully merging this pull request may close these issues.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.