remove `sub_relations` from the `InferCtxt` #119989

lcnr · 2024-01-15T10:50:47Z

While doing so, I tried to remove the delay_span_bug in rematch_impl again, which lead me to discover another freshen bug, fixing that one in the second commit. See commit descriptions for the reasoning behind each change.

r? @compiler-errors

rustbot · 2024-01-15T10:50:56Z

Type relation code was changed

cc @compiler-errors, @lcnr

lcnr · 2024-01-15T10:51:12Z

@bors try @rust-timer queue

bors · 2024-01-15T10:52:22Z

⌛ Trying commit d99a07d with merge 5d00403...

remove `sub_relations` from the `InferCtxt` see commit descriptions for the reasoning of each change. r? `@compiler-errors`

lcnr · 2024-01-15T10:52:30Z

compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs

@@ -1493,10 1493,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 if self.next_trait_solver()
 && let ty::Alias(..) = ty.kind()
 {
- match self
+ // We need to use a separate variable here as we otherwise the temporary for


Suggested change

// We need to use a separate variable here as we otherwise the temporary for

// We need to use a separate variable here as otherwise the temporary for

bors · 2024-01-15T12:17:43Z

☀️ Try build successful - checks-actions
Build commit: 5d00403 (5d004039df46bb665b8ed6385386e3b1b007a6a8)

rustbot · 2024-01-15T14:06:36Z

Some changes occurred to the core trait solver

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

lcnr · 2024-01-15T14:33:51Z

compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs

+ span,
+ E0275,
+ "overflow setting `{a}` to a subtype of `{b}`",
+ )


we could even change this to always emit "cyclic type of infinite size", even though that isn't completely correct as the subtype obligation could simply be unfortunate and the actual overflow cause is something else 🤷 don't have a strong opinion here

rust-timer · 2024-01-15T17:00:09Z

Finished benchmarking commit (5d00403): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

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

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.2%	[-0.4%, -0.2%]	20
Improvements ✅ (secondary)	-0.4%	[-0.6%, -0.2%]	19
All ❌✅ (primary)	-0.2%	[-0.4%, -0.2%]	20

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.6%	[0.8%, 2.4%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

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

Binary size

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

Bootstrap: 669.714s -> 667.26s (-0.37%)
Artifact size: 308.18 MiB -> 308.20 MiB (0.01%)

lcnr · 2024-01-16T07:18:11Z

@craterbot check

craterbot · 2024-01-16T07:18:19Z

👌 Experiment pr-119989 created and queued.
🤖 Automatically detected try build 5d00403
⚠️ Try build based on commit d99a07d, but latest commit is 344cd47. Did you forget to make a new try build?
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2024-01-17T14:54:53Z

🚧 Experiment pr-119989 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

bors · 2024-01-18T19:56:28Z

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

craterbot · 2024-01-19T09:09:44Z

🎉 Experiment pr-119989 is completed!
📊 53 regressed and 5 fixed (408433 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

compiler/rustc_infer/src/infer/error_reporting/sub_relations.rs

we don't track them when canonicalizing or when freshening, resulting in instable caching in the old solver, and issues when instantiating query responses in the new one.

Without doing so we use the same candidate cache entry for `?0: Trait<?1>` and `?0: Trait<?0>`. These goals are different and we must not use the same entry for them.

compiler-errors

r=me when green

lcnr · 2024-02-22T17:27:52Z

@bors r=compiler-errors rollup=never

bors · 2024-02-22T17:27:55Z

📌 Commit c71484e has been approved by compiler-errors

It is now in the queue for this repository.

bors · 2024-02-22T20:45:28Z

⌛ Testing commit c71484e with merge 397937d...

bors · 2024-02-22T22:43:43Z

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 397937d to master...

rust-timer · 2024-02-23T02:17:50Z

Finished benchmarking commit (397937d): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.3%	[-0.5%, -0.2%]	28
Improvements ✅ (secondary)	-0.4%	[-0.7%, -0.2%]	28
All ❌✅ (primary)	-0.3%	[-0.5%, -0.2%]	28

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

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

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

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

Binary size

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

Bootstrap: 650.786s -> 649.393s (-0.21%)
Artifact size: 310.96 MiB -> 310.95 MiB (-0.00%)

matthiaskrgr · 2024-02-23T09:12:58Z

might have fixed #84727

yotamofek · 2024-03-22T18:36:58Z

Apparently, it also broke cargo test for nalgebra. I used cargo-bisect-rustc and it pointed to this PR. This is the (new) type inference error:

error[E0283]: type annotations needed for `matrix::Matrix<T, base::dimension::Const<2>, base::dimension::Const<2>, array_storage::ArrayStorage<T, 2, 2>>`
   --> src/linalg/symmetric_eigen.rs:363:17
    |
363 |             let m = Matrix2::new_random();
    |                 ^   --------------------- type must be known at this point
    |
    = note: cannot satisfy `_: base::scalar::Scalar`
note: required by a bound in `construction::<impl matrix::Matrix<T, R, C, <default_allocator::DefaultAllocator as base::allocator::Allocator<T, R, C>>::Buffer>>::new_random`
   --> src/base/construction.rs:647:9
    |
639 |         pub fn new_random($($args: usize),*) -> Self
    |                ---------- required by a bound in this associated function
...
647 | impl<T: Scalar, R: DimName, C: DimName> OMatrix<T, R, C>
    |         ^^^^^^ required by this bound in `construction::<impl Matrix<T, R, C, <DefaultAllocator as Allocator<T, R, C>>::Buffer>>::new_random`
help: consider giving `m` an explicit type, where the type for type parameter `T` is specified
    |
363 |             let m: matrix::Matrix<T, base::dimension::Const<2>, base::dimension::Const<2>, array_storage::ArrayStorage<T, 2, 2>> = Matrix2::new_random();
    |                                                                                                                                 

error[E0283]: type annotations needed
   --> src/linalg/symmetric_eigen.rs:364:23
    |
364 |             let m = m * m.transpose();
    |                       ^ cannot infer type for struct `matrix::Matrix<_, base::dimension::Const<2>, base::dimension::Const<2>, array_storage::ArrayStorage<_, 2, 2>>`
    |
note: multiple `impl`s satisfying `matrix::Matrix<_, base::dimension::Const<2>, base::dimension::Const<2>, array_storage::ArrayStorage<_, 2, 2>>: Mul<matrix::Matrix<_, base::dimension::Const<2>, base::dimension::Const<2>, array_storage::ArrayStorage<_, 2, 2>>>` found
   --> src/base/ops.rs:458:9
    |
458 | /         impl<T, R: Dim, C: Dim, S> $Trait<T> for Matrix<T, R, C, S>
459 | |             where T: Scalar   $bound,
460 | |                   S: Storage<T, R, C>,
461 | |                   DefaultAllocator: Allocator<T, R, C> {
    | |______________________________________________________^
...
509 |   componentwise_scalarop_impl!(Mul, mul, ClosedMul; MulAssign, mul_assign);
    |   ------------------------------------------------------------------------ in this macro invocation
...
607 | / impl<T, R1: Dim, C1: Dim, R2: Dim, C2: Dim, SA, SB> Mul<Matrix<T, R2, C2, SB>>
608 | |     for Matrix<T, R1, C1, SA>
609 | | where
610 | |     T: Scalar   Zero   One   ClosedAdd   ClosedMul,
...   |
613 | |     DefaultAllocator: Allocator<T, R1, C2>,
614 | |     ShapeConstraint: AreMultipliable<R1, C1, R2, C2>,
    | |_____________________________________________________^
    = note: this error originates in the macro `componentwise_scalarop_impl` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0283`.

The fix for nalgebra is simple, but I'm just wondering if this is something that should have been picked up by crater, and fixed before this PR was merged.

lcnr · 2024-04-02T11:54:49Z

for relnotes: this is small change to our type inference with the intent to cleanup the type system and to simplify future development.

wrt to #119989 (comment), opened a topic on zulip to maybe build tests when running crater in check mode. https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/crater.3A.20tests.20in.20check.20builds

Pkgsrc changes: * Adapt checksums and patches, some have beene intregrated upstream. Upstream chnages: Version 1.78.0 (2024-05-02) =========================== Language -------- - [Stabilize `#[cfg(target_abi = ...)]`] (rust-lang/rust#119590) - [Stabilize the `#[diagnostic]` namespace and `#[diagnostic::on_unimplemented]` attribute] (rust-lang/rust#119888) - [Make async-fn-in-trait implementable with concrete signatures] (rust-lang/rust#120103) - [Make matching on NaN a hard error, and remove the rest of `illegal_floating_point_literal_pattern`] (rust-lang/rust#116284) - [static mut: allow mutable reference to arbitrary types, not just slices and arrays] (rust-lang/rust#117614) - [Extend `invalid_reference_casting` to include references casting to bigger memory layout] (rust-lang/rust#118983) - [Add `non_contiguous_range_endpoints` lint for singleton gaps after exclusive ranges] (rust-lang/rust#118879) - [Add `wasm_c_abi` lint for use of older wasm-bindgen versions] (rust-lang/rust#117918) This lint currently only works when using Cargo. - [Update `indirect_structural_match` and `pointer_structural_match` lints to match RFC] (rust-lang/rust#120423) - [Make non-`PartialEq`-typed consts as patterns a hard error] (rust-lang/rust#120805) - [Split `refining_impl_trait` lint into `_reachable`, `_internal` variants] (rust-lang/rust#121720) - [Remove unnecessary type inference when using associated types inside of higher ranked `where`-bounds] (rust-lang/rust#119849) - [Weaken eager detection of cyclic types during type inference] (rust-lang/rust#119989) - [`trait Trait: Auto {}`: allow upcasting from `dyn Trait` to `dyn Auto`] (rust-lang/rust#119338) Compiler -------- - [Made `INVALID_DOC_ATTRIBUTES` lint deny by default] (rust-lang/rust#111505) - [Increase accuracy of redundant `use` checking] (rust-lang/rust#117772) - [Suggest moving definition if non-found macro_rules! is defined later] (rust-lang/rust#121130) - [Lower transmutes from int to pointer type as gep on null] (rust-lang/rust#121282) Target changes: - [Windows tier 1 targets now require at least Windows 10] (rust-lang/rust#115141) - [Enable CMPXCHG16B, SSE3, SAHF/LAHF and 128-bit Atomics in tier 1 Windows] (rust-lang/rust#120820) - [Add `wasm32-wasip1` tier 2 (without host tools) target] (rust-lang/rust#120468) - [Add `wasm32-wasip2` tier 3 target] (rust-lang/rust#119616) - [Rename `wasm32-wasi-preview1-threads` to `wasm32-wasip1-threads`] (rust-lang/rust#122170) - [Add `arm64ec-pc-windows-msvc` tier 3 target] (rust-lang/rust#119199) - [Add `armv8r-none-eabihf` tier 3 target for the Cortex-R52] (rust-lang/rust#110482) - [Add `loongarch64-unknown-linux-musl` tier 3 target] (rust-lang/rust#121832) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Bump Unicode to version 15.1.0, regenerate tables] (rust-lang/rust#120777) - [Make align_offset, align_to well-behaved in all cases] (rust-lang/rust#121201) - [PartialEq, PartialOrd: document expectations for transitive chains] (rust-lang/rust#115386) - [Optimize away poison guards when std is built with panic=abort] (rust-lang/rust#100603) - [Replace pthread `RwLock` with custom implementation] (rust-lang/rust#110211) - [Implement unwind safety for Condvar on all platforms] (rust-lang/rust#121768) - [Add ASCII fast-path for `char::is_grapheme_extended`] (rust-lang/rust#121138) Stabilized APIs --------------- - [`impl Read for &Stdin`] (https://doc.rust-lang.org/stable/std/io/struct.Stdin.html#impl-Read-for-&Stdin) - [Accept non `'static` lifetimes for several `std::error::Error` related implementations] (rust-lang/rust#113833) - [Make `impl<Fd: AsFd>` impl take `?Sized`] (rust-lang/rust#114655) - [`impl From<TryReserveError> for io::Error`] (https://doc.rust-lang.org/stable/std/io/struct.Error.html#impl-From-for-Error) These APIs are now stable in const contexts: - [`Barrier::new()`] (https://doc.rust-lang.org/stable/std/sync/struct.Barrier.html#method.new) Cargo ----- - [Stabilize lockfile v4](rust-lang/cargo#12852) - [Respect `rust-version` when generating lockfile] (rust-lang/cargo#12861) - [Control `--charset` via auto-detecting config value] (rust-lang/cargo#13337) - [Support `target.<triple>.rustdocflags` officially] (rust-lang/cargo#13197) - [Stabilize global cache data tracking] (rust-lang/cargo#13492) Misc ---- - [rustdoc: add `--test-builder-wrapper` arg to support wrappers such as RUSTC_WRAPPER when building doctests] (rust-lang/rust#114651) Compatibility Notes ------------------- - [Many unsafe precondition checks now run for user code with debug assertions enabled] (rust-lang/rust#120863) This change helps users catch undefined behavior in their code, though the details of how much is checked are generally not stable. - [riscv only supports split_debuginfo=off for now] (rust-lang/rust#120518) - [Consistently check bounds on hidden types of `impl Trait`] (rust-lang/rust#121679) - [Change equality of higher ranked types to not rely on subtyping] (rust-lang/rust#118247) - [When called, additionally check bounds on normalized function return type] (rust-lang/rust#118882) - [Expand coverage for `arithmetic_overflow` lint] (rust-lang/rust#119432) Internal Changes ---------------- These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools. - [Update to LLVM 18](rust-lang/rust#120055) - [Build `rustc` with 1CGU on `x86_64-pc-windows-msvc`] (rust-lang/rust#112267) - [Build `rustc` with 1CGU on `x86_64-apple-darwin`] (rust-lang/rust#112268) - [Introduce `run-make` V2 infrastructure, a `run_make_support` library and port over 2 tests as example] (rust-lang/rust#113026) - [Windows: Implement condvar, mutex and rwlock using futex] (rust-lang/rust#121956)

rustbot assigned compiler-errors Jan 15, 2024

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 Jan 15, 2024

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 15, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 15, 2024

Auto merge of rust-lang#119989 - lcnr:sub_relations-bye-bye, r=<try>

5d00403

remove `sub_relations` from the `InferCtxt` see commit descriptions for the reasoning of each change. r? `@compiler-errors`

lcnr commented Jan 15, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

rustbot added the WG-trait-system-refactor The Rustc Trait System Refactor Initiative label Jan 15, 2024

lcnr commented Jan 15, 2024

View reviewed changes

lcnr force-pushed the sub_relations-bye-bye branch from 25eb092 to 344cd47 Compare January 15, 2024 14:34

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 15, 2024

craterbot added the S-waiting-on-crater Status: Waiting on a crater run to be completed. label Jan 16, 2024

lcnr force-pushed the sub_relations-bye-bye branch from 344cd47 to 42a3816 Compare January 19, 2024 06:44

craterbot removed the S-waiting-on-crater Status: Waiting on a crater run to be completed. label Jan 19, 2024

compiler-errors reviewed Feb 20, 2024

View reviewed changes

compiler/rustc_infer/src/infer/error_reporting/sub_relations.rs Show resolved Hide resolved

Dylan-DPC 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 Feb 21, 2024

lcnr added 4 commits February 22, 2024 17:29

remove sub_relations from infcx, recompute in diagnostics

91535ad

we don't track them when canonicalizing or when freshening, resulting in instable caching in the old solver, and issues when instantiating query responses in the new one.

freshen: resolve root vars

f392a87

Without doing so we use the same candidate cache entry for `?0: Trait<?1>` and `?0: Trait<?0>`. These goals are different and we must not use the same entry for them.

overflow errors: change source to a concrete enum

f7cdff8

do not use <: in subtyping overflow msg

49dc0f2

lcnr force-pushed the sub_relations-bye-bye branch from c150d04 to 49dc0f2 Compare February 22, 2024 16:48

change error messages to be incorrect, but more helpful

c71484e

compiler-errors approved these changes Feb 22, 2024

View reviewed changes

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 Feb 22, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 22, 2024

bors merged commit 397937d into rust-lang:master Feb 22, 2024
12 checks passed

rustbot added this to the 1.78.0 milestone Feb 22, 2024

lcnr deleted the sub_relations-bye-bye branch February 22, 2024 22:55

lcnr mentioned this pull request Feb 23, 2024

ICE #84727

Closed

yotamofek mentioned this pull request Mar 22, 2024

Fix type inference error in tests on rustc beta dimforge/nalgebra#1372

Merged

compiler-errors mentioned this pull request Mar 31, 2024

regression: overflow evaluating the requirement &_ well-formed #123278

Closed

lcnr added the relnotes Marks issues that should be documented in the release notes of the next release. label Apr 2, 2024

lcnr mentioned this pull request Jun 4, 2024

Allow constraining opaque types during various unsizing casts #125610

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remove `sub_relations` from the `InferCtxt` #119989

remove `sub_relations` from the `InferCtxt` #119989

lcnr commented Jan 15, 2024 •

edited

Loading

rustbot commented Jan 15, 2024

lcnr commented Jan 15, 2024

This comment has been minimized.

bors commented Jan 15, 2024

lcnr Jan 15, 2024

bors commented Jan 15, 2024

This comment has been minimized.

rustbot commented Jan 15, 2024

lcnr Jan 15, 2024 •

edited

Loading

rust-timer commented Jan 15, 2024

lcnr commented Jan 16, 2024

craterbot commented Jan 16, 2024

craterbot commented Jan 17, 2024

bors commented Jan 18, 2024

craterbot commented Jan 19, 2024

compiler-errors left a comment

lcnr commented Feb 22, 2024

bors commented Feb 22, 2024

bors commented Feb 22, 2024

bors commented Feb 22, 2024

rust-timer commented Feb 23, 2024

matthiaskrgr commented Feb 23, 2024

yotamofek commented Mar 22, 2024

lcnr commented Apr 2, 2024

	// We need to use a separate variable here as we otherwise the temporary for
	// We need to use a separate variable here as otherwise the temporary for

remove sub_relations from the InferCtxt #119989

remove sub_relations from the InferCtxt #119989

Conversation

lcnr commented Jan 15, 2024 • edited Loading

rustbot commented Jan 15, 2024

lcnr commented Jan 15, 2024

This comment has been minimized.

bors commented Jan 15, 2024

Choose a reason for hiding this comment

bors commented Jan 15, 2024

This comment has been minimized.

rustbot commented Jan 15, 2024

lcnr Jan 15, 2024 • edited Loading

Choose a reason for hiding this comment

rust-timer commented Jan 15, 2024

Overall result: ✅ improvements - no action needed

lcnr commented Jan 16, 2024

craterbot commented Jan 16, 2024

craterbot commented Jan 17, 2024

bors commented Jan 18, 2024

craterbot commented Jan 19, 2024

compiler-errors left a comment

Choose a reason for hiding this comment

lcnr commented Feb 22, 2024

bors commented Feb 22, 2024

bors commented Feb 22, 2024

bors commented Feb 22, 2024

rust-timer commented Feb 23, 2024

Overall result: ✅ improvements - no action needed

matthiaskrgr commented Feb 23, 2024

yotamofek commented Mar 22, 2024

lcnr commented Apr 2, 2024

remove `sub_relations` from the `InferCtxt` #119989

remove `sub_relations` from the `InferCtxt` #119989

lcnr commented Jan 15, 2024 •

edited

Loading

lcnr Jan 15, 2024 •

edited

Loading