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

Prepare for impending LLVM 17 release #4014

Merged
merged 8 commits into from Sep 14, 2023

Conversation

CaseyCarter
Copy link
Member

@CaseyCarter CaseyCarter commented Sep 7, 2023

I tested with LLVM 17.0.0-rc4 and found a couple of issues / regressions:

  • Instead of fixing the friend declaration issues as we expected, they get worse in Clang 17. I relabeled the workaround in <ranges> with LLVM-61763 and added another similar workaround for zip_transform_view.
  • Clang seems to be confused by the deduced return type of our implementation of forward_like (LLVM-64029)

@CaseyCarter CaseyCarter added the enhancement Something can be improved label Sep 7, 2023
@github-actions github-actions bot added this to Work In Progress in Code Reviews Sep 7, 2023
@CaseyCarter CaseyCarter marked this pull request as ready for review September 7, 2023 18:47
@CaseyCarter CaseyCarter requested a review from a team as a code owner September 7, 2023 18:47
@CaseyCarter CaseyCarter moved this from Work In Progress to Initial Review in Code Reviews Sep 7, 2023
@CaseyCarter CaseyCarter moved this from Initial Review to Final Review in Code Reviews Sep 7, 2023
Code Reviews automation moved this from Final Review to Work In Progress Sep 7, 2023
stl/inc/utility Outdated Show resolved Hide resolved
stl/inc/utility Outdated Show resolved Hide resolved
@CaseyCarter CaseyCarter moved this from Work In Progress to Final Review in Code Reviews Sep 7, 2023
stl/inc/utility Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej moved this from Final Review to Ready To Merge in Code Reviews Sep 7, 2023
* Move `_Maybe_const` up from `<ranges>` into `<type_traits>`
* Use `is_rvalue_reference_v<T&&>` instead of `is_lvalue_reference_v<T>`. I'm certain they are complements - so it doesn't matter for correctness - but the former more closely matches the Standard wording which makes things easier for code readers.
stl/inc/ranges Show resolved Hide resolved
Code Reviews automation moved this from Ready To Merge to Work In Progress Sep 8, 2023
stl/inc/utility Outdated Show resolved Hide resolved
@CaseyCarter CaseyCarter moved this from Work In Progress to Final Review in Code Reviews Sep 8, 2023
@CaseyCarter

This comment was marked as outdated.

@azure-pipelines

This comment was marked as outdated.

@CaseyCarter

This comment was marked as resolved.

@azure-pipelines

This comment was marked as resolved.

Copy link
Contributor

@strega-nil-ms strega-nil-ms left a comment

Choose a reason for hiding this comment

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

I prefer the updated forward_like anyways.

@StephanTLavavej StephanTLavavej moved this from Final Review to Ready To Merge in Code Reviews Sep 8, 2023
@StephanTLavavej StephanTLavavej self-assigned this Sep 13, 2023
@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 c3336fb into microsoft:main Sep 14, 2023
35 checks passed
Code Reviews automation moved this from Ready To Merge to Done Sep 14, 2023
@StephanTLavavej
Copy link
Member

Thanks for scouting out LLVM 17! 🏞️ 🥾 😻

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
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants