Skip to content

fix: well-formed goals for tuples, arrays, and slices in next-solver - #23490

Closed
GauravPawar101 wants to merge 0 commit into
rust-lang:masterfrom
GauravPawar101:fix/issue-23475
Closed

GauravPawar101 wants to merge 0 commit into
rust-lang:masterfrom
GauravPawar101:fix/issue-23475

Conversation

@GauravPawar101

@GauravPawar101 GauravPawar101 commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Summary
Fixes #23475

changes

  • crates/hir-ty/src/next_solver/wf.rs (N): WfPredicates visitor over Terms like ruswtc.
  • solver.rs: Implimented visitor.
  • infer/expr.rs, infer/unify.rs: register WF obligation.
  • infer/errors.rs: error-type fallback so the server no longer crashes.
  • dyn_compatibility.rs: region bound helper using caller’s interner.

NEW

  • solver_errors.r, hir/src/diagnostics.rs: solver diagnosticse E0308.
  • ide-diagnostics/src/handlers: extended E0308 handler.
  • 11 new tests matching rustc

VALIDATION

[x] cargo test
[x] cargo clippy, cargo fmt

Disclosure

Claude was used for rustc citations and understanding.

Rustc ref

The implementation is modeled directly on:

  • compiler/rustc_trait_selection/src/traits/wf.rs
  • unnormalized_obligations
  • WfPredicates::add_wf_preds_for_term, require_sized, nominal_obligations
  • add_wf_preds_for_dyn_ty, add_wf_preds_for_pat_ty
  • closure / coroutine-closure / FnDef / Dynamic treatment
  • ConstEvaluatable, ConstArgHasType, WellFormed on inference variables
  • compiler/rustc_hir_typeck/src/expr.rs
  • call-site path WF handling in check_expr_path / instantiate_value_path
  • ObligationCauseCode::WellFormed(WellFormedLoc::...) usage for argument/generic-argument obligations
  • compiler/rustc_hir_analysis/src/check/wfcheck.rs
  • item-level WF entry points that use rustc_trait_selection::traits::wf::obligations
  • The rustc-side object_region_bounds helper referenced from the dyn WF code
    (rustc_middle/rustc_trait_selection dyn-object bounds machinery)

Divergences

following remains unimplemented as FIXME or narrower:

  • Item-level WF is not implemented: too much.
  • [u8; "a"]: existing const-evaluation gap, unchanged.
  • missing items as “assume dispatchable/WF” to avoid false-positive diagnostic.
  • commented as FIXME

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 8, 2026
@rustbot

This comment has been minimized.

@ChayimFriedman2

Copy link
Copy Markdown
Contributor

I suspected that you used AI tools contrary to our policy. Please read our AI policy and follow it.

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 8, 2026
@GauravPawar101 GauravPawar101 reopened this Oct 8, 2026
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 8, 2026
@GauravPawar101

Copy link
Copy Markdown
Contributor Author

@ChayimFriedman2 please check and mind my english, mostly just for a valid md and spelling correction. No code gen, if any problem. Please tell
Thank you for your time!!

@ChayimFriedman2

Copy link
Copy Markdown
Contributor

https://github.com/rust-lang/rust-analyzer/blob/master/AI_POLICY.md

When using AI to author changes to analysis - the code responsible for analyzing Rust code and not for implementing IDE features, including
but not limited to: type inference, MIR, name resolution, macro expansion - generally anything in the crates parser, mbe, hir-expand, hir-def, hir-ty,
although there are exceptions; including when using AI only to analyze bugs and not to write code, you are required to include a citation
of the rustc code responsible for the change you did, along with an explanation of how your change follows from it in case this is not immediately clear.

The reason for that is that it is almost impossible to be fully correct in analysis if we implement things differently from rustc. We should not guess
how to fix bugs in analysis without looking at the rustc code.

@rustbot

This comment has been minimized.

@GauravPawar101

Copy link
Copy Markdown
Contributor Author

check @ChayimFriedman2

@ShoyuVanilla ShoyuVanilla left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Besides the AI policy, I think this doesn't match with what rustc does. It creates wf goals by TypeVisitor and I think it's not that complex to be ported here. (I'm not sure on the perf effect though)

The actual behaviors don't match even for tuples and arrays. As in, if you are making wf goals for ((usize, i32), u8), rustc emits no goals, as it visits each nested scalar types inside the tuples and they are trivially wf. But your code adds Wf((usize, i32)) and Wf(u8). Of course they hold trivially but it's waste to create and evaluate them and sometimes the difference between rustc could be even observable, e.g. by hitting recursion limits for very complex types.

View changes since this review

Comment thread crates/hir-ty/src/next_solver/solver.rs Outdated
}
Some(goals)
}
TyKind::Array(t, _) | TyKind::Slice(t) => Some(vec![

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should check whether const N is usize

@GauravPawar101

GauravPawar101 commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor Author

Hey, quicwk heads up:

  • I kept it minimal to the scope of the bug and flat.
  • Not visitor, as I believe it needs triage, can cause problems.
  • full visitor is O(depth), ours is O(1). minimal in most cases tho
    Fixing other issues.
    Thank you for taking your time to review this :)

@ShoyuVanilla

Copy link
Copy Markdown
Member

Well, this code is just wrong and can cause other bugs or problems by itself, as I've stated above.
And the big O of the visitor is relatively zero no matter how N is large compared to registering and proving those excessive goals caused by this PR, which may require extra heavy operations like canonicalizations or caching.

@ChayimFriedman2

Copy link
Copy Markdown
Contributor

Indeed, don't drift from rustc. We need full WF checking anyway.

@rustbot

rustbot commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (possibly #23181) made this pull request unmergeable. Please resolve the merge conflicts.

@GauravPawar101

Copy link
Copy Markdown
Contributor Author

Yeah, drafted raising pr soon

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 14, 2026
@rustbot

rustbot commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@GauravPawar101
GauravPawar101 deleted the fix/issue-23475 branch October 14, 2026 18:05
@GauravPawar101

GauravPawar101 commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor Author

Moved to #23516 for cleaner discussion and commit history

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.

Another issue like #23114 (but not the same cause)

4 participants