Skip to content

fix: check uncertain subst when unsized coerce #20608

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

fix: check uncertain subst when unsized coerce #20608

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion crates/hir-ty/src/infer/coerce.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use std::iter;

use chalk_ir::{BoundVar, Mutability, TyKind, TyVariableKind, cast::Cast};
use hir_def::{hir::ExprId, lang_item::LangItem};
use rustc_next_trait_solver::solve::HasChanged;
use rustc_type_ir::solve::Certainty;
use stdx::always;
use triomphe::Arc;
Expand Down Expand Up @@ -716,7 +717,7 @@ impl<'db> InferenceTable<'db> {
let goal: Goal = coerce_unsized_tref.cast(Interner);

self.commit_if_ok(|table| match table.solve_obligation(goal) {
Ok(Certainty::Yes) => Ok(()),
Ok((_, Certainty::Yes) | (HasChanged::Yes, Certainty::Maybe(_))) => Ok(()),
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the late reply. Has been quite busy on migrating more things to next-solver 😅

I think we shouldn't accept all ambiguous solutions here. If we success here, we skip for the remaining structural coercion attempts tried after this, so this might regress other type inferences or be false negative on type mismatches.

IMO we have to check extra conditions for the trait solve result like in rustc

or try this even better WIP implementation rust-lang/rust#141926

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this simply because what r-a used to do in chalk based trait solver, which accpets incomplete but not identical subst. It's far from perfect though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But it seems that rustc current impl isn't too hard to follow.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, the previous rust-analyzer had been doing so but I hope we could do better 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But r-a doesn't have any proof tree related infra at the moment.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, we have to implement it, too.
Actually, that's why I worked on #20578 before solving this issue.

cc https://rust-lang.zulipchat.com/#narrow/channel/185405-t-compiler.2Frust-analyzer/topic/.5Bnext-solver.5D.3A.20unsize.20coercions/near/535289423

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, I setup the proof tree infra in my (to be published) coercion PR.

_ => Err(TypeError),
})?;

Expand Down
8 changes: 5 additions & 3 deletions crates/hir-ty/src/infer/unify.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -728,11 +728,13 @@ impl<'a> InferenceTable<'a> {
}

#[tracing::instrument(level = "debug", skip(self))]
pub(crate) fn solve_obligation(&mut self, goal: Goal) -> Result<Certainty, NoSolution> {
pub(crate) fn solve_obligation(
&mut self,
goal: Goal,
) -> Result<(HasChanged, Certainty), NoSolution> {
let goal = InEnvironment::new(&self.trait_env.env, goal);
let goal = goal.to_nextsolver(self.interner);
let result = next_trait_solve_in_ctxt(&self.infer_ctxt, goal);
result.map(|m| m.1)
next_trait_solve_in_ctxt(&self.infer_ctxt, goal)
}

pub(crate) fn register_obligation(&mut self, predicate: Predicate<'a>) {
Expand Down
4 changes: 2 additions & 2 deletions crates/hir-ty/src/tests/coercion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,8 +505,8 @@ fn test() {
//^ S<i8, i16>
let obj: &dyn Bar<_, i8, i16> = &S;
//^ S<i8, i16>
//let obj: &dyn Foo<i8, _> = &S;
// S<{unknown}, {unknown}>
let obj: &dyn Foo<i8, usize> = &S;
//^ S<i8, {unknown}>
}"#,
);
}
Expand Down
Loading