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

Casey's accumulated nitpicks #4945

Merged
merged 10 commits into from
Sep 12, 2024
Merged

Casey's accumulated nitpicks #4945

merged 10 commits into from
Sep 12, 2024

Conversation

CaseyCarter
Copy link
Member

@CaseyCarter CaseyCarter commented Sep 10, 2024

Some minor things that aren't worth individual PRs or CIs. These are almost entirely non-functional changes.

  • Pretty clang-format find/version check messages: Let's make these more consistent with the messages for find_package(Python) and the assembler. Showing the path may help folks with problems.
  • Remove extraneous element_type from ContiguousIterator: While we're here, let's use value_type and difference_type where appropriate so we can see which int is which.
  • Remove unused include from P2163R3_invoke_r: This test doesn't need is_permissive.hpp after Adjust tests relying on CWG-1351 noexcept behavior #4914.
  • Investigate a couple of libc++ failures.
  • Add some missing #endif comments: These were incorrectly removed by Begin removing EDG workaround from ranges::to tests #4944.
  • Change "ASAN" to the canonical "ASan" in expected_results.txt comments.
  • Remove an imprecise comment on the primary template of formatter suggesting that a deleted default constructor is sufficient for a disabled specialization.
  • Cleanup unintended test configuration names. The internal "Runall" test runner interprets trailing comments on configuration lines as providing a name to refer to that configuration in run reports. The STL conventionally avoids such names, forcing Runall to tell us the actual command line instead.

Let's make these more consistent with the messages for `find_package(Python)` and the assembler. Showing the path may help folks with problems.
While we're here, let's use `value_type` and `difference_type` where appropriate so we can see which `int` is which.
This test doesn't need `is_permissive.hpp` after microsoft#4914.
@CaseyCarter CaseyCarter added the documentation Related to documentation or comments label Sep 10, 2024
@CaseyCarter CaseyCarter requested a review from a team as a code owner September 10, 2024 05:11
stl/inc/xutility Outdated Show resolved Hide resolved
@AlexGuteniev

This comment was marked as resolved.

Change occurrences of "ASAN" in `expected_results.txt` to "ASan", except for the occurrence in an ALLCAPS heading.

Drive-by: Change "XFAILs" to "XFAILS" in an ALLCAPS heading.
The deleted default constructor isn't the only feature necessary for a disabled specialization of `formatter`.
The escape hatch variations I think are clear enough and easily searchable, so I removed the comments. I kept the comments in `GH_000431_lex_compare_memcmp_classify`, it's explaining something subtle.
@CaseyCarter

This comment was marked as resolved.

@CaseyCarter CaseyCarter removed their assignment Sep 10, 2024
@StephanTLavavej StephanTLavavej self-assigned this Sep 10, 2024
@StephanTLavavej StephanTLavavej removed their assignment Sep 10, 2024
@StephanTLavavej StephanTLavavej self-assigned this Sep 11, 2024
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo (if I can chain it to the toolset update without causing problems) - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 73b5791 into microsoft:main Sep 12, 2024
39 checks passed
@StephanTLavavej
Copy link
Member

Thanks for keeping the codebase neat and tidy! 💎 ✨ 😻

@CaseyCarter CaseyCarter deleted the misc branch September 12, 2024 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Related to documentation or comments
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants