Skip to content

Remove unnecessary substitution of type params when generating vtable methods. #11386

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

Merged
merged 1 commit into from
Jan 9, 2014

Conversation

rcatolino
Copy link
Contributor

So, like I mentioned in issue #10955 it doesn't seem like we need to call ty::subst_tps when the method is generic. But then I realized that this function doesn't mutate any of its input, and the return value is unused. Plus the type param substitution seems to be taken care of in trans_fn_ref_with_vtables, so I thought I'd just try to remove it. As far as I can tell everything works.

This closes #10955.

@alexcrichton
Copy link
Member

Could you add a test for this as well? You can just follow the other examples in the similar directory.

Sadly I'm not at all familiar with this code, so someone else will have to take a look. @pcwalton or @nikomatsakis ?

vtable methods before translating the ref to the method itself.
@rcatolino
Copy link
Contributor Author

Test added.

bors added a commit that referenced this pull request Jan 9, 2014
So, like I mentioned in issue #10955 it doesn't seem like we need to call ```ty::subst_tps``` when the method is generic. But then I realized that this function doesn't mutate any of its input, and the return value is unused. Plus the type param substitution seems to be taken care of in ```trans_fn_ref_with_vtables```, so I thought I'd just try to remove it. As far as I can tell everything works.

This closes #10955.
@bors bors closed this Jan 9, 2014
@bors bors merged commit 1812a7b into rust-lang:master Jan 9, 2014
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.

index out of bounds: the len is 1 but the index is 1
4 participants