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

<valarray>: Make valarray ADL-proof as required #5157

Merged
merged 2 commits into from
Dec 5, 2024

Conversation

frederick-vs-ja
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja commented Nov 29, 2024

Towards #140 and #3100.

IMO it's quite insane to use an ADL-incompatible type with valarray (or any numeric utility that expects overloaded operators). However, LWG-3133 relaxed the requirements for "numeric types" so much that ADL-bombs became acceptable. This PR also arguably implements LWG-3133 (by using std::addressof instead of operator&).

@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner November 29, 2024 10:50
@CaseyCarter CaseyCarter added the LWG Library Working Group issue label Nov 30, 2024
@CaseyCarter
Copy link
Member

From the old pre-GitHub spreadsheet we used to track LWG issues, it looks like we miscategorized LWG-3133 as a pure wording change requiring no updates to product code. Nice catch.

@CaseyCarter
Copy link
Member

I pushed a commit to include <utility> in GH_000140_adl_proof_construction and replaced a few casts with fully qualified std::forward and std::move calls for clarity.

@StephanTLavavej StephanTLavavej self-assigned this Dec 3, 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 79137b6 into microsoft:main Dec 5, 2024
39 checks passed
@StephanTLavavej
Copy link
Member

Thanks for fixing our favoritest and bestest data structure! After deque, of course. 😹 🤪 💚

@frederick-vs-ja frederick-vs-ja deleted the adl-proof-valarray branch December 5, 2024 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LWG Library Working Group issue
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants