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

rustc: added a check for builtin bounds to vtable search #13228

Closed
wants to merge 1 commit into from

Conversation

dmski
Copy link
Contributor

@dmski dmski commented Mar 31, 2014

There wasn't a check for builtin bounds when looking up vtables, only for trait bounds. This led to types sometimes implementing traits they shouldn't have been able to.

Closes #10751

@@ -124,6 +126,26 @@ fn lookup_vtables_for_param(vcx: &VtableContext,
is_early: bool) -> vtable_param_res {
let tcx = vcx.tcx();

if !type_param_bounds.builtin_bounds.is_empty() {
match fixup_ty(vcx, span, ty, is_early) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Although, I expect this to break with the opt-in work. Would you mind adding a FIXME(#13231) so that I remember to update this code once the new opt-in work is done?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@flaper87
Copy link
Contributor

This looks quite good. The fix will have to be updated by the opt-in bound traits work.

@nikomatsakis r?

// implementation of a trait

trait A {}
impl<T: 'static> A for T {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please extend this test for other built-in bounds too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done as well. I've also added a couple of positive test cases - to make sure opt-in changes don't break them.

bors added a commit that referenced this pull request Apr 18, 2014
There wasn't a check for builtin bounds when looking up vtables, only for trait bounds. This led to types sometimes implementing traits they shouldn't have been able to.

Closes #10751
@dmski dmski closed this Apr 23, 2014
@dmski
Copy link
Contributor Author

dmski commented Apr 23, 2014

This is now failing for me because of some DST changes. Considering that opt-in built-in traits are probably just around the corner now, i'm closing this for now and will re-open with a rebase when opt-in stuff lands (if these changes will still be needed then).

notriddle pushed a commit to notriddle/rust that referenced this pull request Sep 20, 2022
Move reference imports filtering into to_proto layer

Follow up to rust-lang/rust-analyzer#13186
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.

'static bound not enforced in borrowed trait coercion
2 participants