Fallible type ops by jdonszelmann · Pull Request #165335 · rust-lang/rust

jdonszelmann · 2026-11-02T10:26:07Z

This PR adds a new TypeOp which is expected to properly report fulfillment errors rather than assert all obligations holds.

This is a single commit extracted from #156018 and #161457. It adds a currently unused api to mir typeck, that's required to implement the move trait, but is on its own also useful for other changes like resolving all cases of #132279

By extracting it, we can lower the review burden on the other PRs

cc: @zannabianca1997

rustbot · 2026-11-02T10:26:12Z

Some changes occurred in engine.rs, potentially modifying the public API of ObligationCtxt.

cc @lcnr

rustbot · 2026-11-02T10:26:16Z

r? @mejrs

rustbot has assigned @mejrs.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler, types
compiler, types expanded to 77 candidates
Random selection from 21 candidates

jdonszelmann · 2026-11-02T10:26:45Z

r? @lcnr or anyone in @rust-lang/initiative-trait-system-refactor

jdonszelmann · 2026-11-02T13:41:20Z

r? @rust-lang/initiative-trait-system-refactor (lcnr isn't on rotation right now, confirmed with them)

jdonszelmann · 2026-11-02T13:42:11Z

r? project-trait-system-refactor

adwinwhite · 2026-11-04T07:34:36Z

 }

+impl<'tcx> EngineError<'tcx> for ScrubbedTraitError<'tcx> {
+    fn try_report_errors(_infcx: &InferCtxt<'tcx>, _errors: ThinVec<Self>) {}


Surprising that try_report_errors doesn't actually report anything?

View changes since the review

it doesn't in this impl of EngineError but it does for FulfullmentError. I've left a comment in the source.

adwinwhite · 2026-11-04T07:40:11Z

+                let errors = ocx.evaluate_obligations_error_on_ambiguity();
+                match errors {
+                    TraitErrors::HasErrors(errors) => {
+                        E::try_report_errors(infcx, errors);


We would duplicate the reporting? try_report_errors and check_potentially_region_dependent_goals can report similar errors 🤔

View changes since the review

when check_potentially_region_dependent_goals emits errors, it returnsErr(). I have not been able to find a single case in which this actually happens, either manually or in the test suite. I think it's a separate concern that maybe, since that's highly dubious....

it probably needs a bug!() or something, or potentially we can just remove this entire call

but at least I don't think we can get duplicate errors

Yeah, it seems check_potentially_region_dependent_goals is currently not exercised, since we silently ignore ambiguity.
But with #164055 , it'll be used.

~~We won't have duplicate errors~~ but the control flow is a bit convoluted. Could you comment in this and_then closure that this is to report non-region dependent errors, or specifically for the FallableCustomTypeOp?

Can Move obligations have nested goals that're region dependent? Then we'll have duplicate errors.

adwinwhite · 2026-11-04T07:48:48Z

@rustbot author

rustbot · 2026-11-04T07:48:53Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

jdonszelmann · 2026-11-04T14:53:16Z

@rustbot review

adwinwhite

Thank you for added comments!
Now I get the motivation is to report Move trait fulfillment errors in borrowck and those are not related to region uniquification.
This seems to violate the invariant that we shall not have obligation errors in borrowck if HIR typeck succeeds. But if it's what the Move design requires, then no problem from me.

View changes since this review

jdonszelmann · 2026-11-05T14:11:38Z

We cannot know these bounds earlier, since it depends on whether we move a value, which we only know in mir.

rustbot · 2026-11-05T14:36:42Z

This PR was rebased onto a different main 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.

adwinwhite · 2026-11-06T01:22:33Z

The duplicate diagnostics concern is non-fatal and we can improve things later.

@bors r+ rollup

rust-bors · 2026-11-06T01:22:37Z

📌 Commit 4400802 has been approved by adwinwhite

It is now in the queue for this repository.

@zannabianca1997

…adwinwhite Fallible type ops This PR adds a new `TypeOp` which is expected to properly report fulfillment errors rather than assert all obligations holds. This is a single commit extracted from rust-lang#156018 and rust-lang#161457. It adds a currently *unused* api to mir typeck, that's required to implement the move trait, but is on its own also useful for other changes like resolving all cases of rust-lang#132279 By extracting it, we can lower the review burden on the other PRs cc: @zannabianca1997

@zannabianca1997

…adwinwhite Fallible type ops This PR adds a new `TypeOp` which is expected to properly report fulfillment errors rather than assert all obligations holds. This is a single commit extracted from rust-lang#156018 and rust-lang#161457. It adds a currently *unused* api to mir typeck, that's required to implement the move trait, but is on its own also useful for other changes like resolving all cases of rust-lang#132279 By extracting it, we can lower the review burden on the other PRs cc: @zannabianca1997

…uwer Rollup of 19 pull requests Successful merges: - #164784 (Introduce OptimizationObjective and fix the entry shim's size attributes) - #165037 (Drop array elements in order after a partial move) - #165078 (Use the `2.38` binutils version in CI) - #159374 (Lower #[no_mangle] to #[symbol_name]/#[link_name]) - #163284 (feat(Reborrow): Allow multiple lifetimes) - #164775 (Tweak output on misused `const` parameter to provide more context and more accurate suggestion) - #165194 (name elided lifetimes in `use<...>` suggestion when `impl Trait` params are present) - #165222 (Anonymize inner binders when checking refining) - #165335 (Fallible type ops) - #165388 (Accept variadic SysV64, Win64, and Aapcs FnAbis in rustc_public) - #165448 (Update `Vec::retain_unstable` docs to say that this method is generally faster than `Vec::retain_mut`) - #165494 (fix Miri ICE on exit with () return type) - #165503 (update Enzyme submodule) - #165505 (DFV: Reject `#[non_exhaustive]` on enum variants with default field values) - #165512 (Fix warnings for the L4Re targets) - #165514 (Remove unused `WhereEqPredicate` and `ScopeId` types) - #165518 (style-guide: Remove ABI explicitness prescription for extern items) - #165519 (Update books) - #165534 (rename MIR_borrow_checking timing scope to less confusing name)

Rollup of 15 pull requests Successful merges: - #165037 (Drop array elements in order after a partial move) - #165550 (Make `Literal::cstr_value` return a `CString`) - #159374 (Lower #[no_mangle] to #[symbol_name]/#[link_name]) - #163284 (feat(Reborrow): Allow multiple lifetimes) - #164775 (Tweak output on misused `const` parameter to provide more context and more accurate suggestion) - #165194 (name elided lifetimes in `use<...>` suggestion when `impl Trait` params are present) - #165222 (Anonymize inner binders when checking refining) - #165335 (Fallible type ops) - #165388 (Accept variadic SysV64, Win64, and Aapcs FnAbis in rustc_public) - #165448 (Update `Vec::retain_unstable` docs to say that this method is generally faster than `Vec::retain_mut`) - #165494 (fix Miri ICE on exit with () return type) - #165505 (DFV: Reject `#[non_exhaustive]` on enum variants with default field values) - #165512 (Fix warnings for the L4Re targets) - #165514 (Remove unused `WhereEqPredicate` and `ScopeId` types) - #165518 (style-guide: Remove ABI explicitness prescription for extern items)

@zannabianca1997

Rollup merge of #165335 - jdonszelmann:fallible-type-ops, r=adwinwhite Fallible type ops This PR adds a new `TypeOp` which is expected to properly report fulfillment errors rather than assert all obligations holds. This is a single commit extracted from #156018 and #161457. It adds a currently *unused* api to mir typeck, that's required to implement the move trait, but is on its own also useful for other changes like resolving all cases of #132279 By extracting it, we can lower the review burden on the other PRs cc: @zannabianca1997

rust-timer · 2026-11-07T10:38:39Z

Note

This PR was benchmarked as part of triage of its containing rollup: triage URL.

Finished benchmarking commit (818c63a): comparison URL.

Overall result: ❌ regressions - please read:

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

If the regression was expected or you think it can be justified,
please write a comment with sufficient written justification, and add
@rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
If you think that you know of a way to resolve the regression, try to create
a new PR with a fix for the regression.
If you do not understand the regression or you think that it is just noise,
you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	1.1%	[0.2%, 3.5%]	108
Regressions ❌ (secondary)	1.4%	[0.2%, 4.2%]	124
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.1%	[0.2%, 3.5%]	108

Max RSS (memory usage)

This perf run didn't have relevant results for this metric.

Cycles

Results (primary 1.7%, secondary 3.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.8%	[2.1%, 3.4%]	8
Regressions ❌ (secondary)	3.6%	[2.3%, 4.8%]	18
Improvements ✅ (primary)	-2.6%	[-2.7%, -2.5%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.7%	[-2.7%, 3.4%]	10

Binary size

This perf run didn't have relevant results for this metric.

Artifact size: 406.27 MiB -> 406.18 MiB (-0.02%)

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 2, 2026

rustbot assigned mejrs Nov 2, 2026

rustbot assigned lcnr and unassigned mejrs Nov 2, 2026

This comment was marked as outdated.

Sign in to view

This comment was marked as off-topic.

Sign in to view

rustbot assigned adwinwhite and unassigned lcnr Nov 2, 2026

adwinwhite reviewed Nov 4, 2026

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 4, 2026

jdonszelmann force-pushed the fallible-type-ops branch from a96573c to c5ec872 Compare November 4, 2026 14:50

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 4, 2026

This comment has been minimized.

Sign in to view

jdonszelmann force-pushed the fallible-type-ops branch from c5ec872 to 616cf33 Compare November 4, 2026 15:00

adwinwhite reviewed Nov 5, 2026

View reviewed changes

fallible type ops

4400802

jdonszelmann force-pushed the fallible-type-ops branch from 616cf33 to 4400802 Compare November 5, 2026 14:36

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 6, 2026

JonathanBrouwer mentioned this pull request Nov 6, 2026

Rollup of 13 pull requests #165540

Closed

JonathanBrouwer mentioned this pull request Nov 6, 2026

Rollup of 19 pull requests #165548

Closed

Zalathar mentioned this pull request Nov 7, 2026

Rollup of 15 pull requests #165579

Merged

rust-bors Bot merged commit 2fc4c29 into rust-lang:main Nov 7, 2026
14 checks passed

rustbot added this to the 1.101.0 milestone Nov 7, 2026

rustbot added the perf-regression Performance regression. label Nov 7, 2026

Conversation

jdonszelmann commented Nov 2, 2026 • edited by adwinwhite Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Nov 2, 2026

Uh oh!

rustbot commented Nov 2, 2026

Uh oh!

jdonszelmann commented Nov 2, 2026

Uh oh!

jdonszelmann commented Nov 2, 2026

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as off-topic.

jdonszelmann commented Nov 2, 2026

Uh oh!

Uh oh!

adwinwhite Nov 4, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adwinwhite Nov 4, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adwinwhite Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

adwinwhite commented Nov 4, 2026

Uh oh!

rustbot commented Nov 4, 2026

Uh oh!

jdonszelmann commented Nov 4, 2026

Uh oh!

This comment has been minimized.

adwinwhite left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jdonszelmann commented Nov 5, 2026

Uh oh!

rustbot commented Nov 5, 2026

Uh oh!

adwinwhite commented Nov 6, 2026

Uh oh!

rust-bors Bot commented Nov 6, 2026

Uh oh!

Uh oh!

rust-timer commented Nov 7, 2026

Overall result: ❌ regressions - please read:

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

jdonszelmann commented Nov 2, 2026 •

edited by adwinwhite

Loading

adwinwhite Nov 4, 2026 •

edited by rustbot

Loading

adwinwhite Nov 4, 2026 •

edited by rustbot

Loading

adwinwhite Nov 5, 2026 •

edited

Loading

adwinwhite left a comment •

edited by rustbot

Loading