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

<functional>: Make default_searcher ADL-proof #4379

Merged

Conversation

frederick-vs-ja
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja commented Feb 9, 2024

Separated from #4004. Towards #140.

This PR makes default_searcher ADL-proof and makes boyer_moore_searcher and boyer_moore_horspool_searcher accept ADL-incompatible functors.

Currently, internal function calls involving program-defined types and std::swap calls are _STD-qualified, while other internal functions (which only involve void* and integers) are not.

There're some difficulties in making boyer_moore_searcher and boyer_moore_horspool_searcher accept ADL-incompatible element types: see #4380.

- ADL-incompatible functors for all searchers, and
- ADL-incompatible value type for `default_searcher`.
@StephanTLavavej StephanTLavavej removed their assignment Feb 9, 2024
@StephanTLavavej
Copy link
Member

Thanks! I added COMPILE-ONLY comments. Technically we don't need to qualify std::size_t and std::hash in a test that's using namespace std; but I figured it was ok to keep the qualification.

@StephanTLavavej StephanTLavavej self-assigned this Feb 12, 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 c674bcb into microsoft:main Feb 13, 2024
35 checks passed
@StephanTLavavej
Copy link
Member

🎉 ✨ 🐈

@frederick-vs-ja frederick-vs-ja deleted the adl-proof-default-searcher branch February 14, 2024 03:38
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
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants