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 inference of indexing argument (partly) #5211

Merged
merged 1 commit into from Jul 3, 2020

Conversation

flodiebold
Copy link
Member

We need to add the T: Index<Arg> obligation to be resolved later as well, otherwise we can't make inferences about Arg later based on the Index impls.

This still doesn't fix indexing with integer variables though; there's a further problem with Chalk floundering because of the variable, I think.

@matklad
Copy link
Member

matklad commented Jul 3, 2020

cargo fmt fails

pub trait Index<Idx> {
type Output;
}
}
Copy link
Member

@matklad matklad Jul 3, 2020

Choose a reason for hiding this comment

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

at some point we need to figure a better way to specify such preludes...

I guess, we can do //- core, and have that auto-magically insert everything, but I wonder if it makes sense to keep stuff more granular?

LGTM!

Copy link
Member Author

Choose a reason for hiding this comment

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

I do kind of like having it explicit what types/traits are actually needed by the test. Also, keeping it small helps when debugging tests.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that's true. But we might do something granular, but still with only one trait Index written down

//- include(std::core::Index, std::result::Result)

but that won't you see at a glance what's going on...

Copy link
Member Author

Choose a reason for hiding this comment

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

That might be nice. Maybe there should just be std/core "snippets" that the tests can include.

We need to add the `T: Index<Arg>` obligation to be resolved later as well,
otherwise we can't make inferences about `Arg` later based on the `Index` impls.

This still doesn't fix indexing with integer variables though; there's a further
problem with Chalk floundering because of the variable, I think.
@flodiebold
Copy link
Member Author

bors r+

@bors
Copy link
Contributor

bors bot commented Jul 3, 2020

@bors bors bot merged commit 7ad7f3c into rust-lang:master Jul 3, 2020
@flodiebold flodiebold deleted the indexing-fix branch July 3, 2020 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants