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

Various cleanups #2140

Merged

Conversation

StephanTLavavej
Copy link
Member

This is a collection of unrelated cleanups, structured as a series of commits for easier reviewing. I've separated out the pure comments/formatting cleanups (#2136), test fixes (#2137), and product fixes (#2138, #2139), leaving these commits which alter syntax but don't significantly change behavior. The user-visible changes are to <stdatomic.h> and (almost invisibly) <experimental/coroutine>.

  • Remove unused __DOUBLE_EXPONENT_BITS/__FLOAT_EXPONENT_BITS.
  • We can directly include <Windows.h> now.
    • This was working around a WinSDK bug which has been fixed. This is our test code, so we don't have to worry about users with old WinSDKs.
  • Change typename CharT to class CharT.
    • This was the only inconsistent occurrence in product code.
  • Stylistically reorder exception_ptr operators; provide only necessary ones for C++20.
    • This should provide the same behavior to users.
  • Construct tags with empty braces.
  • Capitalize template parameters.
  • Change size() == 0 to empty() for strings.
  • Remove unnecessary parens in (b.func) (b.data).
  • Avoid unnecessary auto.
  • libcxx: Unskip C++23 tests (will be "unsupported" until tests.py should map /std:c++latest to 'c++2b' #2001 is fixed).
  • Emit an error when compiling stdatomic.h as C.
  • Simplify stoi(): int and long are the same size for us.
  • Use deprecated attributes in <experimental/coroutine> (improves syntax highlighting).
    • This slightly clarifies the messages emitted to users. I didn't think it was worth centralizing these in yvals_core.h, or numbering them.
  • Demacroize <xthreads.h> size/alignment constants.
    • These were already _Ugly instead of _UGLY_MACROS. There are other macro constants but I didn't want to perform invasive renamings for them at this time.

@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Aug 18, 2021
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner August 18, 2021 20:57
stl/inc/stdatomic.h Show resolved Hide resolved
@@ -95,7 +95,7 @@ _NODISCARD inline int stoi(const string& _Str, size_t* _Idx = nullptr, int _Base
_Xinvalid_argument("invalid stoi argument");
}

if (_Errno_ref == ERANGE || _Ans < INT_MIN || INT_MAX < _Ans) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will this break icc? I mean I don't care, but that may be why we checked this in the first place

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question (as we try to avoid gratuitous breaks). It shouldn't - the Intel C++ Compiler issues we've encountered have involved 80-bit long double (and occasionally new compiler intrinsics). AFAIK, there are no issues with our LLP64 assumption that sizeof(int) equals sizeof(long) equals 4 (that assumption is widespread throughout our codebase).

This was originally being checked because MSVC's STL was licensed from Dinkumware, where the upstream source code was portable to many platforms.

@StephanTLavavej
Copy link
Member Author

I'm mirroring this to an MSVC-internal PR. Please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 18d8f37 into microsoft:main Aug 27, 2021
@StephanTLavavej StephanTLavavej deleted the cleanups5-other-improvements branch August 27, 2021 01:31
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