Skip to content

-Znext-solver allow ExprKind::Call for not-yet defined opaques #145993

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

Jump to bottom
Merged
merged 2 commits into from
Sep 18, 2025
Merged

-Znext-solver allow ExprKind::Call for not-yet defined opaques #145993

merged 2 commits into from
Sep 18, 2025

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Aug 29, 2025 •
edited
Loading

Based on #146329. Revival of #140496. See the comment on OpaqueTypesJank. I've used the following document while working on this https://hackmd.io/Js61f8PRTcyaiyqS-fH9iQ.

Fixes rust-lang/trait-system-refactor-initiative#181. It does introduce one subtle footgun we may want to handle before stabilization, opened rust-lang/trait-system-refactor-initiative#230 for that. Also cc rust-lang/trait-system-refactor-initiative#231 for deref and index operations

r? @BoxyUwU

@rustbot
Copy link
Collaborator

rustbot commented Aug 29, 2025

BoxyUwU is currently at their maximum review capacity.
They may take a while to respond.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Aug 29, 2025
@lcnr lcnr mentioned this pull request Aug 29, 2025
Closed
@lcnr lcnr changed the title -Znext-solver allow ExprKind::Call on not-yet defined opaques -Znext-solver allow ExprKind::Call for not-yet defined opaques Aug 29, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
lcnr
lcnr commented Aug 29, 2025
View reviewed changes
@bors
Copy link
Collaborator

bors commented Aug 30, 2025

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

@lcnr lcnr force-pushed the allow-calling-opaques branch from 9ccb5e9 to 279018c Compare September 11, 2025 10:13
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@lcnr lcnr force-pushed the allow-calling-opaques branch from 279018c to 102a7d6 Compare September 11, 2025 11:08
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@lcnr lcnr force-pushed the allow-calling-opaques branch 6 times, most recently from 219519c to c9f782a Compare September 12, 2025 12:43
lcnr
lcnr commented Sep 12, 2025
View reviewed changes
@lcnr lcnr force-pushed the allow-calling-opaques branch 2 times, most recently from dc1b010 to efde093 Compare September 15, 2025 12:39
@lcnr
Copy link
Contributor Author

lcnr commented Sep 15, 2025

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

Sign in to view
@lcnr lcnr marked this pull request as ready for review September 15, 2025 12:39
rust-bors bot added a commit that referenced this pull request Sep 15, 2025
@rust-bors
Auto merge of #145993 - lcnr:allow-calling-opaques, r=<try>
cc9a9f2 
`-Znext-solver` allow `ExprKind::Call` for not-yet defined opaques
@rust-bors

This comment has been minimized.

Sign in to view
@rustbot
Copy link
Collaborator

rustbot commented Sep 15, 2025

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (cc9a9f2): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

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)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -0.2% [-0.2%, -0.2%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (secondary -1.3%)

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

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -1.3% [-1.3%, -1.3%] 1
All ❌✅ (primary) - - 0

Cycles

Results (secondary 1.3%)

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

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 2.6% [2.3%, 2.8%] 3
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -2.3% [-2.3%, -2.3%] 1
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 473.872s -> 471.595s (-0.48%)
Artifact size: 390.16 MiB -> 390.18 MiB (0.00%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 15, 2025
@lcnr lcnr force-pushed the allow-calling-opaques branch from eaea480 to dcf560a Compare September 15, 2025 17:27
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@lcnr lcnr force-pushed the allow-calling-opaques branch from dcf560a to c8a3348 Compare September 16, 2025 07:17
BoxyUwU
BoxyUwU reviewed Sep 17, 2025
View reviewed changes
compiler/rustc_hir_typeck/src/callee.rs
return None;
}

ty::Infer(ty::TyVar(vid)) => {
Copy link
Member

Choose a reason for hiding this comment

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

feels subtle to me that we are specifically checking for TyVar instead of just all ty::Infer? How does the rest of the code handle int/float vars

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can never impl Fn and Deref, so 🤷 we generally accept int/float infer vars, e.g. structurally_resolve_type also does so

compiler/rustc_hir_analysis/src/autoderef.rs
// To do so we don't eagerly bail if the current type is the hidden type of an
// opaque type and instead return `None` in `fn overloaded_deref_ty` if the
// opaque does not have a `Deref` item-bound.
if let &ty::Infer(ty::TyVar(vid)) = self.state.cur_ty.kind()
Copy link
Member

Choose a reason for hiding this comment

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

similar here, I realise we previously only checked is_ty_var, but how does the rest of this codepath handle int/float vars

Copy link
Contributor Author

Choose a reason for hiding this comment

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

they don't implement Deref so they are the final self type and users of autoderef have to handle them if necessary

compiler/rustc_hir_typeck/src/place_op.rs Outdated
imm_tr,
base_ty,
opt_rhs_ty,
TreatNotYetDefinedOpaques::AsInfer,
Copy link
Member

Choose a reason for hiding this comment

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

i guess its the same reasoning here?

compiler/rustc_hir_typeck/src/place_op.rs Outdated
mut_tr,
base_ty,
opt_rhs_ty,
TreatNotYetDefinedOpaques::AsInfer,
Copy link
Member

Choose a reason for hiding this comment

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

i guess its the same reasoning here?

@BoxyUwU BoxyUwU 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 Sep 17, 2025
@lcnr lcnr force-pushed the allow-calling-opaques branch from c8a3348 to 173233e Compare September 18, 2025 10:53
@lcnr lcnr force-pushed the allow-calling-opaques branch from 173233e to 9913c47 Compare September 18, 2025 10:58
@lcnr
Copy link
Contributor Author

lcnr commented Sep 18, 2025

@bors r=BoxyUwU :3

@bors
Copy link
Collaborator

bors commented Sep 18, 2025

📌 Commit 9913c47 has been approved by BoxyUwU

It is now in the queue for this repository.

@bors bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 18, 2025
@bors
Copy link
Collaborator

bors commented Sep 18, 2025

⌛ Testing commit 9913c47 with merge 4cd91ef...

@lcnr lcnr mentioned this pull request Sep 18, 2025
Draft
@bors
Copy link
Collaborator

bors commented Sep 18, 2025

☀️ Test successful - checks-actions
Approved by: BoxyUwU
Pushing 4cd91ef to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 18, 2025
@bors bors merged commit 4cd91ef into rust-lang:master Sep 18, 2025
11 checks passed
@rustbot rustbot added this to the 1.92.0 milestone Sep 18, 2025
@github-actions GitHub Actions
Copy link
Contributor

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 32e3d9f (parent) -> 4cd91ef (this PR)

Test differences

Show 45 test diffs

Stage 1

  • [ui] tests/ui/impl-trait/non-defining-uses/ambiguous-ops.rs#current: [missing] -> pass (J1)
  • [ui] tests/ui/impl-trait/non-defining-uses/ambiguous-ops.rs#next: [missing] -> pass (J1)
  • [ui] tests/ui/impl-trait/non-defining-uses/call-expr-incorrect-choice-diagnostics.rs#current: [missing] -> pass (J1)
  • [ui] tests/ui/impl-trait/non-defining-uses/call-expr-incorrect-choice-diagnostics.rs#next: [missing] -> pass (J1)
  • [ui] tests/ui/impl-trait/non-defining-uses/deref-constrains-self-ty.rs#current: [missing] -> pass (J1)
  • [ui] tests/ui/impl-trait/non-defining-uses/deref-constrains-self-ty.rs#next: [missing] -> pass (J1)
  • [ui] tests/ui/impl-trait/non-defining-uses/function-call-on-infer.rs#current: [missing] -> pass (J1)
  • [ui] tests/ui/impl-trait/non-defining-uses/function-call-on-infer.rs#next: [missing] -> pass (J1)
  • [ui] tests/ui/impl-trait/non-defining-uses/impl-deref-function-call.rs#current: [missing] -> pass (J1)
  • [ui] tests/ui/impl-trait/non-defining-uses/impl-deref-function-call.rs#next: [missing] -> pass (J1)
  • [ui] tests/ui/impl-trait/non-defining-uses/shex_compat-regression-test.rs#current: [missing] -> pass (J1)
  • [ui] tests/ui/impl-trait/non-defining-uses/shex_compat-regression-test.rs#next: [missing] -> pass (J1)

Stage 2

  • [ui] tests/ui/impl-trait/non-defining-uses/ambiguous-ops.rs#current: [missing] -> pass (J0)
  • [ui] tests/ui/impl-trait/non-defining-uses/ambiguous-ops.rs#next: [missing] -> pass (J0)
  • [ui] tests/ui/impl-trait/non-defining-uses/call-expr-incorrect-choice-diagnostics.rs#current: [missing] -> pass (J0)
  • [ui] tests/ui/impl-trait/non-defining-uses/call-expr-incorrect-choice-diagnostics.rs#next: [missing] -> pass (J0)
  • [ui] tests/ui/impl-trait/non-defining-uses/deref-constrains-self-ty.rs#current: [missing] -> pass (J0)
  • [ui] tests/ui/impl-trait/non-defining-uses/deref-constrains-self-ty.rs#next: [missing] -> pass (J0)
  • [ui] tests/ui/impl-trait/non-defining-uses/function-call-on-infer.rs#current: [missing] -> pass (J0)
  • [ui] tests/ui/impl-trait/non-defining-uses/function-call-on-infer.rs#next: [missing] -> pass (J0)
  • [ui] tests/ui/impl-trait/non-defining-uses/impl-deref-function-call.rs#current: [missing] -> pass (J0)
  • [ui] tests/ui/impl-trait/non-defining-uses/impl-deref-function-call.rs#next: [missing] -> pass (J0)
  • [ui] tests/ui/impl-trait/non-defining-uses/shex_compat-regression-test.rs#current: [missing] -> pass (J0)
  • [ui] tests/ui/impl-trait/non-defining-uses/shex_compat-regression-test.rs#next: [missing] -> pass (J0)

Additionally, 21 doctest diffs were found. These are ignored, as they are noisy.

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 4cd91ef8223ef54111d21aa9e9e71b3b26477dd3 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. aarch64-apple: 5458.3s -> 6428.0s (17.8%)
  2. dist-android: 1571.8s -> 1301.8s (-17.2%)
  3. x86_64-gnu-llvm-20: 2374.1s -> 2766.1s (16.5%)
  4. aarch64-gnu-debug: 3726.1s -> 4337.8s (16.4%)
  5. dist-x86_64-apple: 8144.5s -> 9396.2s (15.4%)
  6. i686-gnu-1: 7141.0s -> 8050.5s (12.7%)
  7. x86_64-gnu-miri: 4400.0s -> 4929.4s (12.0%)
  8. dist-aarch64-windows-gnullvm: 4717.7s -> 5249.8s (11.3%)
  9. arm-android: 5683.0s -> 6272.9s (10.4%)
  10. aarch64-gnu-llvm-20-1: 3284.3s -> 3616.9s (10.1%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

This was referenced Sep 18, 2025
Open
Open
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (4cd91ef): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

@rustbot label: -perf-regression

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)		 - - 0
Regressions ❌
(secondary)		 0.1% [0.1%, 0.1%] 1
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -0.2% [-0.3%, -0.0%] 3
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (secondary 3.8%)

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

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 4.6% [4.1%, 5.0%] 7
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -2.0% [-2.0%, -2.0%] 1
All ❌✅ (primary) - - 0

Cycles

Results (primary -2.1%, secondary -2.6%)

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

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 3.0% [3.0%, 3.0%] 1
Improvements ✅
(primary)		 -2.1% [-2.1%, -2.1%] 1
Improvements ✅
(secondary)		 -3.7% [-4.6%, -2.6%] 5
All ❌✅ (primary) -2.1% [-2.1%, -2.1%] 1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 471.73s -> 473.718s (0.42%)
Artifact size: 387.92 MiB -> 389.93 MiB (0.52%)

This was referenced Sep 18, 2025
Open
Merged
@lcnr lcnr deleted the allow-calling-opaques branch September 18, 2025 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BoxyUwU BoxyUwU BoxyUwU left review comments

Assignees

@BoxyUwU BoxyUwU

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Milestone
1.92.0
Development

Successfully merging this pull request may close these issues.

calling impl Fn in defining scope
6 participants
@lcnr @rustbot @rust-log-analyzer @bors @rust-timer @BoxyUwU