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 #3177

Merged
merged 10 commits into from
Oct 26, 2022
Merged

Various cleanups #3177

merged 10 commits into from
Oct 26, 2022

Conversation

StephanTLavavej
Copy link
Member

@StephanTLavavej StephanTLavavej commented Oct 25, 2022

These non-overlapping changes are structured as a series of fine-grained commits for easy reviewing but are collected into a single PR to reduce the load on the CI system.

  • <xlocinfo>, <xlocnum>: The _Stof and _Stofx families are unused, so we can remove their declarations from stl/inc.
    • xstof.cpp, xstod.cpp, xstold.cpp: We can comment their definitions as preserved for bincompat in stl/src.
    • xwstof.cpp, xwstod.cpp, xwstold.cpp: Also comment the wide definitions, which didn't have declarations.
    • Note that all of these definitions are marked as _CRTIMP2_PURE, so they'll continue to be exported.
  • <xlocinfo>: _GetLocaleForCP was declared but never defined, so we can drop this extremely complicated declaration.
  • <string>: Qualify _STD getline().
    • This revealed a more subtle problem: the first overload being changed here was calling the second (unintentionally relying on ADL 🙀 because the second hadn't been declared yet), also spending an unnecessary function call. Adding _STD move(_Istr) fixes this, and makes all of these overloads call the actual implementation above.
  • special_math.cpp: Now that we've merged update boost-math and fix std::ellint_2 #3077 to always use the standalone boost-math submodule, we can simplify this:
  • P0896R4_ranges_alg_is_permutation/test.cpp: Nitpick, we conventionally avoid defining a struct and a variable simultaneously. Define NonCopyableBool and b separately.
  • Skip a libcxx test due to a possible path length issue.
    • This is consistently failing on my local machine. I suspect this was introduced when Update LLVM #2976 updated LLVM, although I didn't verify that.
  • Remove compiler bug workarounds for LLVM-46269 which was fixed in Clang 15.
    • Thanks to @CaseyCarter for providing the desired definition of payload_destructor, also removing an unnecessary semicolon.
  • azure-devops/cmake-configure-build.yml: Followup to Update google-benchmark to 1.7.0 #3151, drop unused variables when configuring the benchmarks. This fixes a warning message printed by Azure Pipelines:
CMake Warning:
    Manually-specified variables were not used by the project:

    LIT_FLAGS
    STL_USE_ANALYZE

StephanTLavavej and others added 9 commits October 24, 2022 15:59
…ompat in stl/src.

They're marked as _CRTIMP2_PURE in stl/src, so they'll continue to be exported.
Also, avoid calling a function before it's been declared (unintentional ADL and an unnecessary function call).
Fixes:

```
CMake Warning:
  Manually-specified variables were not used by the project:

    LIT_FLAGS
    STL_USE_ANALYZE
```
@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Oct 25, 2022
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner October 25, 2022 17:28
Copy link
Member

@CaseyCarter CaseyCarter left a comment

Choose a reason for hiding this comment

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

For posterity: I validated there are no other occurrences of LLVM-46269 in the STL after applying this change.

@StephanTLavavej StephanTLavavej self-assigned this Oct 25, 2022
@StephanTLavavej
Copy link
Member Author

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

@StephanTLavavej
Copy link
Member Author

I've pushed an additional commit to restore C4702 silencing for the internal chpe build.

@StephanTLavavej StephanTLavavej merged commit 296e840 into microsoft:main Oct 26, 2022
@StephanTLavavej StephanTLavavej deleted the cleanups branch October 26, 2022 13:14
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