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 miscellaneous changes #4900

Merged
merged 6 commits into from
Aug 25, 2024

Conversation

CaseyCarter
Copy link
Member

@CaseyCarter CaseyCarter commented Aug 20, 2024

A grab bag of minor changes broken down for quick-and-easy commit-wise review:

  • <ranges>: simplify zip_view::_Size_closure by making it a static constexpr data member instead of a function
  • Use conventional LLVM issue linking style
  • Simplify _Tuple_like metaprogramming: implements _Tuple_like_impl as a variable template with four partial specializations instead of as a disjunction of four other variable template specializations. Consequently, each evaluation of _Tuple_like causes the compiler to memoize one constexpr variable instead of five.
  • Don't return a decayed boolean-testable since the exposition-only concept doesn't allow for construction of an object of decayed type from a model of boolean-testable. Note that the changes in erase_if(forward_list, pred) and erase_if(list, pred) are the proposed resolution of LWG-4135, which I feel is a no-brainer and can be implemented without comments. I audited for other occurrences after noticing we do the same thing in forward_list::remove and list::remove.
    The fold expressions in <ranges> are a weird case: they are only problematic when the pack has one element. For zero or more than one elements, the fold expression has type bool, but with exactly one element it's a boolean-testable that the lambda implicitly decays on return. I think it's a bit more elegant to fix these by adding || false to the fold expressions to make them binary than to explicitly specify the return type of the lambda.
  • Don't return in cartesian_product_view::_Iterator::iter_swap. We can return an expression with void type in a function with return type void, but we typically don't do so when the types are concrete.

I could be convinced that the boolean-testable change deserves to be in a separate PR with some test coverage.

CaseyCarter and others added 5 commits August 20, 2024 12:04
... by making it a `static constexpr` data member instead of a function
Implements `_Tuple_like_impl` as a variable template with four partial specializations instead of as a disjunction of four other variable template specializations. Consequently, each evaluation of `_Tuple_like` causes the compiler to memoize one constexpr variable instead of five.
... since the exposition-only concept doesn't allow for construction of an object of decayed type from a model of *boolean-testable*. Note that the changes in `erase_if(forward_list, pred)` and `erase_if(list, pred)` are the proposed resolution of LWG-4135, which I feel is a no-brainer and can be implemented without comments. I audited for other occurrences after noticing we do the same thing in `forward_list::remove` and `list::remove`.

The fold expressions in `<ranges>` are a weird case: they are only problematic when the pack has one element. For zero or more than one elements, the fold expression has type `bool`, but with exactly one element it's a *boolean-testable* that the lambda implicitly decays on return. I think it's a bit more elegant to fix these by adding `|| false` to the fold expressions to make them binary than to explicitly specify the return type of the lambda.
We _can_ `return` an expression with `void` type in a function with return type `void`, but we typically don't do so when the types are concrete.
@CaseyCarter CaseyCarter added the enhancement Something can be improved label Aug 20, 2024
@CaseyCarter CaseyCarter requested a review from a team as a code owner August 20, 2024 19:11
@StephanTLavavej StephanTLavavej self-assigned this Aug 20, 2024
@StephanTLavavej StephanTLavavej removed their assignment Aug 21, 2024
@StephanTLavavej StephanTLavavej self-assigned this Aug 25, 2024
@StephanTLavavej
Copy link
Member

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

@StephanTLavavej StephanTLavavej merged commit 7782684 into microsoft:main Aug 25, 2024
39 checks passed
@StephanTLavavej
Copy link
Member

Yay various cleanups! 🧹 🎉 🐈‍⬛

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
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants