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

ICE on trait method with type parameter bounded by trait with lifetime parameter #12856

Closed
dwrensha opened this issue Mar 13, 2014 · 4 comments · Fixed by #13261
Closed

ICE on trait method with type parameter bounded by trait with lifetime parameter #12856

dwrensha opened this issue Mar 13, 2014 · 4 comments · Fixed by #13261
Labels
A-lifetimes Area: Lifetimes / regions A-traits Area: Trait system

Comments

@dwrensha
Copy link
Contributor

I think this relates to #12807 and #5121.

The following program causes rustc to crash:

// lifetimes_bug.rs
use std;

pub struct StructReader<'a> {
    marker : std::kinds::marker::ContravariantLifetime<'a>,
}

pub trait FromStructReader<'a> {
    fn new(struct_reader : StructReader<'a>) -> Self;
}

pub trait MessageReader {
    fn get_root<'a, T :FromStructReader<'a>>(&'a self) -> T;
}

pub fn main () {}
$ rustc lifetimes_bug.rs 
error: internal compiler error: No inferred index entry for unknown node (id=31)
@dwrensha
Copy link
Contributor Author

cc @pnkfelix

@Kimundi
Copy link
Member

Kimundi commented Mar 20, 2014

Another testcase:

trait RefIterable<T> {
    fn refs<'a, Iter: Iterator<&'a T>>(&'a self) -> Iter;
}
fn main() {}
error: internal compiler error: No inferred index entry for unknown node (id=18)
note: the compiler hit an unexpected failure path. this is a bug.
note: we would appreciate a bug report: http://static.rust-lang.org/doc/master/complement-bugreport.html
note: run with `RUST_BACKTRACE=1` for a backtrace
task 'rustc' failed at '~Any', /build/rust-git/src/rust/src/libsyntax/diagnostic.rs:123

@nikomatsakis
Copy link
Contributor

cc me

bors added a commit that referenced this issue Apr 17, 2014
Fix #12856.

I wanted to put this up first because I wanted to get feedback about the second commit in the series, commit 8599236.  Its the more invasive part of the patch and is largely just belt-and-suspenders assertion checking; in the commit message I mentioned at least one other approach we could take here.  Or we could drop the belt-and-suspenders and just rely on the guard added in the first patch, commit 8d6a005 (which is really quite trivial on its own).

So any feedback on what would be better is appreciated.

r? @nikomatsakis
@dwrensha
Copy link
Contributor Author

Thanks for fixing!

As far as I know, now #12857 is the only thing preventing capnproto-rust from having a safe interface.

fasterthanlime pushed a commit to fasterthanlime/rust that referenced this issue Jul 26, 2022
feat: Spawn a proc-macro-srv instance per workspace

cc rust-lang/rust-analyzer#12855

The idea is to have each server be spawned with the appropriate toolchain, that way workspaces with differing toolchains shouldn't suffer from proc-macro abi mismatches.
dingxiangfei2009 pushed a commit to dingxiangfei2009/rust that referenced this issue Jul 28, 2024
This commit fixes a bug introduced in rust-lang#12706, where the behavior of the
lint has been changed, to avoid suggestions that introduce a move. The
motivation in the commit message is quite poor (if the detection for
significant drops is not sufficient because it's not transitive, the
proper fix would be to make it transitive). However, rust-lang#12454, the linked
issue, provides a good reason for the change — if the value being
borrowed is bound to a variable, then moving it will only introduce
friction into future refactorings.

Thus rust-lang#12706 changes the logic so that the lint triggers if the value
being borrowed is Copy, or is the result of a function call, simplifying
the logic to the point where analysing "is this the only use of this
value" isn't necessary.

However, said PR also introduces an undocumented carveout, where
referents that themselves are mutable references are treated as Copy,
to catch some cases that we do want to lint against. However, that is
not sound — it's possible to consume a mutable reference by moving it.

To avoid emitting false suggestions, this PR reintroduces the
referent_used_exactly_once logic and runs that check for referents that
are themselves mutable references.

Thinking about the code shape of &mut x, where x: &mut T, raises the
point that while removing the &mut outright won't work, the extra
indirection is still undesirable, and perhaps instead we should suggest
reborrowing: &mut *x. That, however, is left as possible future work.

Fixes rust-lang#12856
dingxiangfei2009 pushed a commit to dingxiangfei2009/rust that referenced this issue Jul 28, 2024
… r=xFrednet

needless_borrows_for_generic_args: Fix for &mut

This commit fixes a bug introduced in rust-lang#12706, where the behavior of the lint has been changed, to avoid suggestions that introduce a move. The motivation in the commit message is quite poor (if the detection for significant drops is not sufficient because it's not transitive, the proper fix would be to make it transitive). However, rust-lang#12454, the linked issue, provides a good reason for the change — if the value being borrowed is bound to a variable, then moving it will only introduce friction into future refactorings.

Thus rust-lang#12706 changes the logic so that the lint triggers if the value being borrowed is Copy, or is the result of a function call, simplifying the logic to the point where analysing "is this the only use of this value" isn't necessary.

However, said PR also introduces an undocumented carveout, where referents that themselves are mutable references are treated as Copy, to catch some cases that we do want to lint against. However, that is not sound — it's possible to consume a mutable reference by moving it.

To avoid emitting false suggestions, this PR reintroduces the referent_used_exactly_once logic and runs that check for referents that are themselves mutable references.

Thinking about the code shape of &mut x, where x: &mut T, raises the point that while removing the &mut outright won't work, the extra indirection is still undesirable, and perhaps instead we should suggest reborrowing: &mut *x. That, however, is left as possible future work.

Fixes rust-lang#12856

changelog: none
Jarcho pushed a commit to Jarcho/rust that referenced this issue Aug 26, 2024
… r=xFrednet

needless_borrows_for_generic_args: Fix for &mut

This commit fixes a bug introduced in rust-lang#12706, where the behavior of the lint has been changed, to avoid suggestions that introduce a move. The motivation in the commit message is quite poor (if the detection for significant drops is not sufficient because it's not transitive, the proper fix would be to make it transitive). However, rust-lang#12454, the linked issue, provides a good reason for the change — if the value being borrowed is bound to a variable, then moving it will only introduce friction into future refactorings.

Thus rust-lang#12706 changes the logic so that the lint triggers if the value being borrowed is Copy, or is the result of a function call, simplifying the logic to the point where analysing "is this the only use of this value" isn't necessary.

However, said PR also introduces an undocumented carveout, where referents that themselves are mutable references are treated as Copy, to catch some cases that we do want to lint against. However, that is not sound — it's possible to consume a mutable reference by moving it.

To avoid emitting false suggestions, this PR reintroduces the referent_used_exactly_once logic and runs that check for referents that are themselves mutable references.

Thinking about the code shape of &mut x, where x: &mut T, raises the point that while removing the &mut outright won't work, the extra indirection is still undesirable, and perhaps instead we should suggest reborrowing: &mut *x. That, however, is left as possible future work.

Fixes rust-lang#12856

changelog: none
Jarcho pushed a commit to Jarcho/rust that referenced this issue Aug 26, 2024
… r=xFrednet

needless_borrows_for_generic_args: Fix for &mut

This commit fixes a bug introduced in rust-lang#12706, where the behavior of the lint has been changed, to avoid suggestions that introduce a move. The motivation in the commit message is quite poor (if the detection for significant drops is not sufficient because it's not transitive, the proper fix would be to make it transitive). However, rust-lang#12454, the linked issue, provides a good reason for the change — if the value being borrowed is bound to a variable, then moving it will only introduce friction into future refactorings.

Thus rust-lang#12706 changes the logic so that the lint triggers if the value being borrowed is Copy, or is the result of a function call, simplifying the logic to the point where analysing "is this the only use of this value" isn't necessary.

However, said PR also introduces an undocumented carveout, where referents that themselves are mutable references are treated as Copy, to catch some cases that we do want to lint against. However, that is not sound — it's possible to consume a mutable reference by moving it.

To avoid emitting false suggestions, this PR reintroduces the referent_used_exactly_once logic and runs that check for referents that are themselves mutable references.

Thinking about the code shape of &mut x, where x: &mut T, raises the point that while removing the &mut outright won't work, the extra indirection is still undesirable, and perhaps instead we should suggest reborrowing: &mut *x. That, however, is left as possible future work.

Fixes rust-lang#12856

changelog: none
MabezDev pushed a commit to esp-rs/rust that referenced this issue Sep 3, 2024
… r=xFrednet

needless_borrows_for_generic_args: Fix for &mut

This commit fixes a bug introduced in rust-lang#12706, where the behavior of the lint has been changed, to avoid suggestions that introduce a move. The motivation in the commit message is quite poor (if the detection for significant drops is not sufficient because it's not transitive, the proper fix would be to make it transitive). However, rust-lang#12454, the linked issue, provides a good reason for the change — if the value being borrowed is bound to a variable, then moving it will only introduce friction into future refactorings.

Thus rust-lang#12706 changes the logic so that the lint triggers if the value being borrowed is Copy, or is the result of a function call, simplifying the logic to the point where analysing "is this the only use of this value" isn't necessary.

However, said PR also introduces an undocumented carveout, where referents that themselves are mutable references are treated as Copy, to catch some cases that we do want to lint against. However, that is not sound — it's possible to consume a mutable reference by moving it.

To avoid emitting false suggestions, this PR reintroduces the referent_used_exactly_once logic and runs that check for referents that are themselves mutable references.

Thinking about the code shape of &mut x, where x: &mut T, raises the point that while removing the &mut outright won't work, the extra indirection is still undesirable, and perhaps instead we should suggest reborrowing: &mut *x. That, however, is left as possible future work.

Fixes rust-lang#12856

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lifetimes Area: Lifetimes / regions A-traits Area: Trait system
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants