Skip to content

Commit

Permalink
Auto merge of rust-lang#17882 - ShoyuVanilla:issue-17866, r=lnicola
Browse files Browse the repository at this point in the history
fix: Panic while canonicalizing erroneous projection type

Fixes rust-lang#17866

The root cause of rust-lang#17866 is quite horrifyng 😨

```rust
trait T {
    type A;
}

type Foo = <S as T>::A; // note that S isn't defined

fn main() {
    Foo {}
}
```

While inferencing alias type `Foo = <S as T>::A`;

https://github.com/rust-lang/rust-analyzer/blob/78c2bdce860dbd996a8083224d01a96660dd6a15/crates/hir-ty/src/infer.rs#L1388-L1398

the error type `S` in it is substituted by inference var in L1396 above as below;

https://github.com/rust-lang/rust-analyzer/blob/78c2bdce860dbd996a8083224d01a96660dd6a15/crates/hir-ty/src/infer/unify.rs#L866-L869

This new inference var's index is `1`, as the type inferecing procedure here previously inserted another inference var into same `InferenceTable`.

But after that, the projection type made from the above then passed to the following function;

https://github.com/rust-lang/rust-analyzer/blob/78c2bdce860dbd996a8083224d01a96660dd6a15/crates/hir-ty/src/traits.rs#L88-L96

here, a whole new `InferenceTable` is made, without any inference var and in the L94, this table calls;

https://github.com/rust-lang/rust-analyzer/blob/78c2bdce860dbd996a8083224d01a96660dd6a15/crates/hir-ty/src/infer/unify.rs#L364-L370

And while registering `AliasEq` `obligation`, this obligation contains inference var `?1` made from the previous table, but this table has only one inference var `?0` made at L365.
So, the chalk panics when we try to canonicalize that obligation to register it, because the obligation contains an inference var `?1` that the canonicalizing table doesn't have.

Currently, we are calling `InferenceTable::new()` to do some normalizing, unifying or coercing things to some targets that might contain inference var that the new table doesn't have.
I think that this is quite dangerous footgun because the inference var is just an index that does not contain the information which table does it made from, so sometimes this "foreign" index might cause panic like this case, or point at the wrong variable.

This PR mitigates such behaviour simply by inserting sufficient number of inference vars to new table to avoid such problem.
This strategy doesn't harm current r-a's intention because the inference vars that passed into new tables are just "unresolved" variables in current r-a, so this is just making sure that such "unresolved" variables exist in the new table
  • Loading branch information
bors committed Aug 14, 2024
2 parents e2f2e73 + 81b68b2 commit 89cd585
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 3 deletions.
3 changes: 2 additions & 1 deletion src/tools/rust-analyzer/crates/hir-ty/src/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1436,7 +1436,8 @@ impl<'a> InferenceContext<'a> {
let remaining = unresolved.map(|it| path.segments()[it..].len()).filter(|it| it > &0);
let ty = match ty.kind(Interner) {
TyKind::Alias(AliasTy::Projection(proj_ty)) => {
self.db.normalize_projection(proj_ty.clone(), self.table.trait_env.clone())
let ty = self.table.normalize_projection_ty(proj_ty.clone());
self.table.resolve_ty_shallow(&ty)
}
_ => ty,
};
Expand Down
21 changes: 21 additions & 0 deletions src/tools/rust-analyzer/crates/hir-ty/src/tests/regression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2141,3 +2141,24 @@ fn test() {
}"#,
);
}

#[test]
fn issue_17866() {
check_infer(
r#"
trait T {
type A;
}
type Foo = <S as T>::A;
fn main() {
Foo {};
}
"#,
expect![[r#"
60..75 '{ Foo {}; }': ()
66..72 'Foo {}': {unknown}
"#]],
);
}
14 changes: 12 additions & 2 deletions src/tools/rust-analyzer/crates/hir-ty/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ use hir_def::{
};
use hir_expand::name::Name;
use intern::sym;
use stdx::panic_context;
use stdx::{never, panic_context};
use triomphe::Arc;

use crate::{
db::HirDatabase, infer::unify::InferenceTable, utils::UnevaluatedConstEvaluatorFolder, AliasEq,
AliasTy, Canonical, DomainGoal, Goal, Guidance, InEnvironment, Interner, ProjectionTy,
ProjectionTyExt, Solution, TraitRefExt, Ty, TyKind, WhereClause,
ProjectionTyExt, Solution, TraitRefExt, Ty, TyKind, TypeFlags, WhereClause,
};

/// This controls how much 'time' we give the Chalk solver before giving up.
Expand Down Expand Up @@ -90,6 +90,16 @@ pub(crate) fn normalize_projection_query(
projection: ProjectionTy,
env: Arc<TraitEnvironment>,
) -> Ty {
if projection.substitution.iter(Interner).any(|arg| {
arg.ty(Interner)
.is_some_and(|ty| ty.data(Interner).flags.intersects(TypeFlags::HAS_TY_INFER))
}) {
never!(
"Invoking `normalize_projection_query` with a projection type containing inference var"
);
return TyKind::Error.intern(Interner);
}

let mut table = InferenceTable::new(db, env);
let ty = table.normalize_projection_ty(projection);
table.resolve_completely(ty)
Expand Down

0 comments on commit 89cd585

Please sign in to comment.