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

Fix begin/end for valarray crashing on empty instance #2435

Merged
merged 3 commits into from
Jan 20, 2022
Merged

Fix begin/end for valarray crashing on empty instance #2435

merged 3 commits into from
Jan 20, 2022

Conversation

Saalvage
Copy link
Contributor

This issue also causes range-based for loops to break for empty valarrays.

I also added regression tests, which I hope are fine.

If there's anything wrong just tell me and I'll happily adjust!

@Saalvage Saalvage requested a review from a team as a code owner December 19, 2021 01:31
@ghost
Copy link

ghost commented Dec 19, 2021

CLA assistant check
All CLA requirements met.

stl/inc/valarray Show resolved Hide resolved
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.

I'm going to apply my suggestions which clean up pre-existing weirdness in <valarray>.

stl/inc/valarray Outdated Show resolved Hide resolved
stl/inc/valarray Outdated Show resolved Hide resolved
stl/inc/valarray Show resolved Hide resolved
tests/tr1/tests/valarray/test.cpp Show resolved Hide resolved
@CaseyCarter CaseyCarter added the bug Something isn't working label Dec 19, 2021
There's no reason to make friend declarations private.
@StephanTLavavej StephanTLavavej self-assigned this Jan 12, 2022
tests/tr1/tests/valarray/test.cpp Show resolved Hide resolved
tests/tr1/tests/valarray/test.cpp Show resolved Hide resolved
@StephanTLavavej StephanTLavavej removed their assignment Jan 19, 2022
@StephanTLavavej StephanTLavavej self-assigned this Jan 19, 2022
@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 20fea3f into microsoft:main Jan 20, 2022
@StephanTLavavej
Copy link
Member

Thanks for fixing this bug - and congratulations on your first microsoft/STL commit! 🎉 😻 🚀

This will ship as part of VS 2022 17.2 Preview 2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
  翻译: