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

<ranges>: Explicitly specify the template parameters for tuple #3609

Merged
merged 1 commit into from
Apr 7, 2023

Conversation

cpplearner
Copy link
Contributor

@cpplearner cpplearner commented Mar 31, 2023

Class template argument deduction for std::tuple does not always result in a tuple of the arguments (e.g. when the first argument is a pair). This PR adds the template parameters to ensure that these uses of tuple behave as expected in pathological cases.

example
#include <ranges>
#include <utility>

struct Tag {};

using Sent = std::pair<Tag, int>;

bool operator==(int*, Sent);
std::ptrdiff_t operator-(int*, Sent);
std::ptrdiff_t operator-(Sent, int*);

struct Rng {
    int* begin();
    Sent end();
};

int main() {
    auto x  = std::views::cartesian_product(Rng{});
    (void) (x.begin() - std::default_sentinel); // `operator-` calls `_It._Distance_from(_It._End_tuple(...))`
    // Before: `_End_tuple` returns a `tuple<Tag, int>`, which results in an error in `_Distance_from`
    // After: `_End_tuple` returns a `tuple<Sent>` as expected
}

@cpplearner cpplearner requested a review from a team as a code owner March 31, 2023 08:02
@StephanTLavavej StephanTLavavej added bug Something isn't working ranges C++20/23 ranges labels Mar 31, 2023
@StephanTLavavej
Copy link
Member

Looks good to me. I'm not concerned about adding test coverage for this extremely pathological scenario.

@StephanTLavavej StephanTLavavej changed the title <ranges>: Explicitly specify the template parameters for tuple <ranges>: Explicitly specify the template parameters for tuple Apr 4, 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 97f5698 into microsoft:main Apr 7, 2023
@StephanTLavavej
Copy link
Member

Thanks for noticing and fixing this problem! 🦅 😸 🛠️

@cpplearner cpplearner deleted the ctad-hatred branch April 7, 2023 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ranges C++20/23 ranges
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants