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

Merged
merged 25 commits into from
Mar 19, 2022
Merged

Conversation

StephanTLavavej
Copy link
Member

@StephanTLavavej StephanTLavavej commented Feb 27, 2022

  • Enable clang-format in many places with zero-to-minor changes.
    • It's getting better at handling concepts.
  • Add newlines between non-chained if statements.
    • This convention makes it easier to see where new chains of tests begin.
  • Change __analysis_assume to _Analysis_assume_.
    • This is for consistency - they're equivalent, but we should use the more "modern" form.
  • <algorithm>: Fix an unpaired // clang-format off after _Generate_n_fn.
  • <complex>: Centralize on using _STD instead of ::std::.
    • This is more searchable, and may matter in the distant future.
  • <complex>: Remove unnecessary _STD qualification on an _Ugly function, _Log_abs().
  • <limits>: Remove unnecessary qualification of std::numeric_limits.
  • <locale>: Remove unnecessary defenses for macroized isalnum etc.
  • <random>: Use range-for.
    • Use _Val, as _Elem is almost always a template parameter here.
  • <random>: Improve formatting of _Refill_lower().
  • <random>: Slightly simplify loop in _Small_poisson_distribution::operator().
    • This slightly changes control flow, but with identical behavior.
    • Use initializers instead of assignments with a comma operator.
    • Directly return instead of break followed by return. (There's no other way to exit this loop.)
    • Scope _Res to the loop, making its counting behavior even clearer.
  • <ranges>: Use extended friend syntax.
    • We prefer this because it's shorter and avoids any possibility of declaring stuff.
  • <yvals_core.h>: Cite <cuchar>: mbrtoc8() and c8rtomb() are not yet implemented #2207 for mbrtoc8() and c8rtomb().
  • <yvals_core.h>: Remove dead macro _STL_WIN32_WINNT_WINXP.
  • Rename 💥 to 🐶 etc., following our current policies.
  • Rename to GH_001411_core_headers/test.compile.pass.cpp.
    • This tells the Python-powered test harness to not bother executing this // COMPILE-ONLY test.
  • P0912R5_coroutine/test.cpp: Simplify __cpp_lib_coroutine test.
  • P0980R1_constexpr_strings/test.cpp: Attach a brace to test_gh_2524().
  • VSO_0102478_moving_allocators/test.cpp: Pass format strings directly to printf.
    • This allows the compiler to detect type mismatches.
  • Delete flaky skipped tr1 tests.
    • Suggested by @AlexGuteniev - we have no intention of salvaging these tests. If we wanted better coverage in these areas, we'd write new tests from scratch.
  • P0083R3_splicing_maps_and_sets/test.cpp: Drop special case for Clang.
    • Suggested by @fsb4000. This test is for C++17 and above. Clang 10 added support for C++20 constinit, so we can drop the special case here. (We don't need special coverage for C++17 specifically here.)
  • Update comments to // TRANSITION, Clang 14 coroutine support.
  • P0912R5_coroutine/env.lst: Add MSVC/EDG C++20 and Clang C++23 coverage.
  • Update comments and test coverage for /await:strict.
    • Thanks to @cpplearner for noticing this. Coroutines are usually C++20 but are also activated by /await:strict, so we permanently need to check __cpp_impl_coroutine - this is not a temporary thing until Clang 14 is released. (Tests that are C++20-specific can be simplified after Clang 14, so they still have TRANSITION comments.) Finally, we should also add a new configuration to VSO_0157762_feature_test_macros.
  • VSO_0157762_feature_test_macros/env.lst: Remove unused configurations.

If desired, I can split this into smaller PRs, but I believe that the changes are simple enough to be reviewed in a single PR.

@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Feb 27, 2022
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner February 27, 2022 01:08
@fsb4000

This comment was marked as resolved.

This test is for C++17 and above. Clang 10 added support for
C++20 constinit, so we can drop the special case here.
(We don't need special coverage for C++17 specifically here.)
@CaseyCarter CaseyCarter removed their assignment Mar 2, 2022
Thanks to @cpplearner for noticing this. Coroutines are usually C++20
but are also activated by `/await:strict`, so we permanently need to
check `__cpp_impl_coroutine` - this is not a temporary thing until
Clang 14 is released. (Tests that are C++20-specific can be simplified
after Clang 14, so they still have TRANSITION comments.) Finally, we
should also add a new configuration to
`VSO_0157762_feature_test_macros`.
These configurations were unused after GH 752 "Remove compiler specific feature test macro tests".
@StephanTLavavej
Copy link
Member Author

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

@StephanTLavavej StephanTLavavej merged commit f310638 into microsoft:main Mar 19, 2022
@StephanTLavavej StephanTLavavej deleted the cleanup-stuff branch March 19, 2022 10:18
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.

6 participants