dyn Trait
unsoundnessNear the end of this month, I spent a large chunk of time trying to patch an unsoundness with dyn Trait
types. I filed an issue for this in #133361 "Coherence with object types with overlapping supertrait projections is incomplete".
I left a brief comment about the problem in the issue, but The tl;dr is that we handle associated type bounds in dyn traits (like "Item = i32
" in dyn Iterator<Item = i32>
) in a funky kind of way when they come from supertrait bounds, like:
/// An iterator that only returns `i32`s
trait ConstrainedIterator: Iterator<Item = i32> {}
impl<T> ConstrainedIterator for T where T: Iterator<Item = i32> {}
// You can now write `dyn ConstrainedIterator` instead of typing
// the `Item` bound like `dyn ConstrainedIterator<Item = i32>`
// (which is redundant).
While at this time (December 1) the full description of the fix is not yet fleshed out (sorry!), I opened a PR evaluating a potential fix in #133397 "Fix dyn incompleteness with multiple supertraits with different substitutions". After tweaking it a bunch, I don't believe there's any major outstanding fallout, but we still need to re-evaluate that by re-running crater on the ecosystem again and writing a report about the nature of the changes.
Along the way, I ended up cleaning up the dyn Trait
lowering code quite a lot, mostly suppressing spurious errors (#133393, #133394, #133362, and #133660).
Also along the way, I discovered another case where associated type bounds act strangely. In #133392 "Fix ICE when multiple supertrait substitutions need assoc but only one is provided", I fixed a compiler crash in codegen:
trait Sup<T> {
type Assoc: Default;
}
impl<T: Default> Sup<T> for () {
type Assoc = T;
}
impl<T: Default, U: Default> Dyn<T, U> for () {}
trait Dyn<A, B>: Sup<A, Assoc = A> + Sup<B> {}
fn main() {
let q: <dyn Dyn<i32, u32> as Sup<u32>>::Assoc = Default::default();
//^^^ 💥 crash 💥 ^^^
}
I don't expect this to affect users, since if code relies on it, it's already very wrong. However, since this is theoretically a breaking change, this required a types team FCP which is currently pending.
This was a huge month in the next-generation trait solver. We have been working towards our goal to "bootstrap"[1] the compiler using the new trait solver, and have finally been able to achieve this goal by stacking on a bunch of changes which have or will be split out into their own PRs soon. This represents a huge milestone in the maturity of the new trait solver, since rustc is a complex program with a lot of dependencies.
Most of the changes that have gone towards bootstrapping the new trait solver have to do with lazy normalization, which (in brief terms) has to do the new trait solver's approach to handling associated types like <[i32; 1] as IntoIterator>::Item
in type-checking. I put up a ton of small tweaks to type-checking code that assumes types have been fully normalized which is not true when we use the new trait solver (#133518, #133520, #133521, #133517, #133558, and #133559).
In #133519 "Check xform_ret_ty for WF in the new solver to improve method winnowing", I fixed a subtle bug having to do with the way we do method lookup. The description may be interesting to those who know more about method probing in rustc, or you can check out my video on method probing[2] in rustc for a more technical deep-dive on method probing in general.
Const traits continue to move forwards since the major overhaul that happened last month.
In #132479 "Yeet the effects feature, move it onto const_trait_impl", I consolidated the #[feature(effects)]
feature gate into the #[feature(const_trait_impl)]
feature gate. The effects feature gate was split out during initial experimentation of the previous major rewrite of const traits, but at this point needing to enable two feature gates is a bit redundant.
I opened #133325 "Reimplement ~const trait specialization" to rework the way we validate specializing const impls, which blocks constifying standard library traits like PartialEq
which use specialization under the hood. The general status of specialization is a bit iffy[3], but we need to have support for it for now.
The PR #133218 "Implement ~const item bounds in RPIT" implements const bounds in return-position impl traits. While I don't expect these to be particularly useful by themselves, I think they will be very useful when paired with const closure support in the future.
#133403 "Make adjust_fulfillment_errors
work with HostEffectPredicate
and const_conditions
" is a small diagnostics improvement for ~const
trait bounds and allows us to point at the right argument that causes a ~const
trait bound to be unsatisfied in a method call.
#133321 "Get rid of HIR const checker" removes an old HIR-based const checking routine that is no longer needed now that we have const trait checking in MIR. As I noted in the PR, improvements to the MIR-based const checker in the mean time have made this redundant, and removing it also removes duplicated error messages.
In #133216 "Implement ~const Fn
trait goal in the new solver" and #132329 "Implement ~const Destruct
effect goal in the new solver", I implemented built-in support for the Fn*
family of traits and the Destruct
trait, the latter of which controls whether things can be dropped in const functions.
Finally, to wrap this up, I constified the Drop
/Destruct
(#133402), and Deref
/DerefMut
traits (#133260) in the standard library. While these aren't particularly useful on their own, they really prove in an end-to-end way that we can begin to implement const traits in the standard library once again.
This was also a big month for async closures. All of the work towards async closures has finally culminated in
#132706 "Stabilize async closures (RFC 3668)". I encourage you to read the stabilization report for that PR, since I put a lot of time and effort into it.
However, before we could get to this point, I also went through and cleaned up a bunch of FIXMEs and small outstanding issues in the async closures implementation (e.g. #132486 and #132488).
We (unfortunately, imo) decided to stabilize async closures using the AsyncFn()
trait family syntax rather than the sugared "async Fn()
" syntax. To prepare for this stabilization, I opened #132611 "Add AsyncFn* to the prelude in all editions" and reworked the way we pretty-print AsyncFn
in the compiler to use the parentheses correctly (#132697 and #132911).
Finally, I broke out the still-experimental async Fn()
syntax into a separate feature gate in #132612 "Gate async fn trait bound modifier on async_trait_bounds". I'd like to work towards stabilizing this syntax too, and later deprecating the AsyncFn()
syntax, but this can proceed independently of stabilizing async closures, since it requires the lang team to reach consensus on this too. My opinion is that delivering async closures is too important to block on a basically arbitrary syntax question like how to spell the Fn
trait bounds.
#133428 "Actually use placeholder regions for trait method late bound regions in collect_return_position_impl_trait_in_trait_tys" fixes a subtle unsoundness that I introduced in the pursuit for better error messages when inferring the types for impl Trait
in trait methods (RPITITs). I'm pretty ashamed I didn't see the issue when I authored the regression PR, because it seems so obvious in hindsight, but luckily the fix was pretty simple to reason about (from a type system perspective).
#132625 "Only disable cache if predicate has opaques within it" fixes a compile-time regression that was due to a soundness fix (#126024) that caused regressions in impl Trait
-heavy code.
Unfortunately, I was made aware of that last issue due to somebody being snarky on Twitter about the regression issue being open for a while with no obvious progress towards fix it. I wanted to take this opportunity to ask Rust community members to have patience towards Rust compiler contributors; our resources are often pretty limited (especially when it comes to things like type system soundness), and we often have to balance things like speed and compiler correctness in ways that often aren't obvious to a user who just wants cargo build
to go as fast as possible. While I encourage users to ping us on issues that they believe have gone stale, especially if it's something like a 10x speed regression in compilation, I don't believe it's ever appropriate to make passive-aggressive tweets about an issue you want solved.
I put some effort towards robustifying some of the Rust 2024 migration lints this month too. The edition is ready for testing in nightly rustc, and I'm super excited for it to land.
In #132817 "Recurse into APITs in impl_trait_overcaptures", I fixed a bug having to do with us not detecting changes to RPIT lifetime capture rules in async fn
that return impl Trait
. In #132816 "Dont suggest use<impl Trait> when we have an edition-2024-related borrowck issue", I fixed erroneous suggestions in the same migration.
In #132933 "Make sure that we suggest turbofishing the right type arg for never suggestion" and #133691 "Check let source before suggesting annotation", I improved the suggestions related to never-type fallback. I caused a crash when implementing those suggestions, though, which I fixed in #132528 "Use *_opt
typeck results fns to not ICE in fallback suggestion".
While this isn't directly related to the edition, in #133482 "Only error raw lifetime followed by ' in edition 2021+" I fixed a theoretical lexer regression having to do with raw lifetimes which landed in 1.83. Raw lifetimes will be used to migrate 'gen
lifetimes, since gen
is being reserved as a keyword in Rust 2024.
The PR #133367 "Simplify array length mismatch error reporting (to not try to turn consts into target usizes)" is both a fix for a crash, and also a diagnostics improvement. We used to have a special-cased diagnostic for array length mismatches; this PR generalizes the message and removes its dependence on const evaluation.
#132489 "Fix closure arg extraction in extract_callable_info, generalize it to async closures" improves the suggestion to add ()
to call a function when a user passes a function directly. The existing logic did not work for async closures.
#132780 "use verbose for path separator suggestion" is a slight tweak to an existing suggestion to be clearer what the change is:
--> $DIR/issue-22692.rs:2:13
|
LL | let _ = String.new();
- | ^^^^^^- help: use the path separator to refer to an item: `::`
+ help: use the path separator to refer to an item
+ |
+ LL | let _ = String::new();
+ | ~~
In #133365 "Make compare_impl_item into a query", I made our "method compatibility" validation more robust to avoid cases where the const evaluator tried to call an invalid impl of a method, like:
trait Foo {
fn generate() -> usize;
}
impl Foo for () {
fn generate() -> &'static str { "hello, world" }
}
…which would cause crashes due to the unexpected mismatch in types between trait and implementation. It fixed quite a few filed issues!
Finally, related to my experimental work on #![feature(non_lifetime_binders)]
(which allows writing things like for<T>
– still super broken and experimental), I put up #132832 "Deny capturing late-bound ty/const params in nested opaques" to fix a bunch of crashes when they are used with impl Trait
.
The PR #132795 "Check use<..> in RPITIT for refinement" implements the only outstanding blocker for stabilizing use<..>
precise-capturing bounds in traits, like:
trait Test {
fn method(&mut self) -> impl Iterator<Item = u32> + use<Self>;
//~^ This method doesn't capture the implicit lifetime in `&mut self`.
}
One of the still major outstanding missing features for async fn in traits is support for dyn Trait
. In #133122 "Add unpolished, experimental support for AFIDT (async fn in dyn trait)", I added experimental support for coercing traits with async fn
s in them if they return boxed futures. After this PR lands, I will experiment with an adapter type that allows automatically boxing the return type of async fn
.
In #133358 "Don't type error if we fail to coerce Pin<T> because it doesnt contain a ref", I fixed a small bug having to do with the new #![feature(pin_ergonomics)]
when dealing with pinned types that aren't &T
/&mut T
.
#132757 "Get rid of check_opaque_type_well_formed" consolidates two different routines that we used to check the validity of impl Trait
types into one. This makes it easier to refactor this code in the future, since it'll need overhauling to support the new trait solver, and generally fixes the confusion about "why do we have two function that basically do the same thing?" – the answer is for no particularly good reason[4].
To support experimentation with dyn* Trait
, I introduced the PointerLike
trait to answer the question of "does this type share the same layout as *const ()
?" which is a requirement to coerce a type into dyn* Trait
. In #133226 "Make PointerLike
opt-in instead of built-in", I made this trait no longer require invoking the layout computation machinery in the trait solver, which avoids query cycles and simplifies the implementation.
#133319 "Simplify fulfill_implication" disentangles some trait solver code having to do with specialization. This code hasn't been touched in quite a long time, and it's great to be able to use all the new functionality we've added to the type system APIs since then.
Finally, we used to have an idiom in the compiler where we'd prefix or suffix all the variants of an enum, for example BoundRegionKind
, with something like Br
(e.g. BrAnon
, BrNamed
, etc.), and then glob-import all of those enum's variants. This leads to two problems: (1.) it's not always clear what enum a variant comes from, and (2.) it leads to namespace pollution. Over the last 10 years in the compiler we've been gradually moving away from this idiom. In #132580 "Remove unnecessary pub enum glob-imports from rustc_middle::ty", I removed this and changed the compiler to instead name the enum "normally" (e.g. BoundRegionKind::Named
).
i.e. build the compiler using itself ↩︎
https://www.youtube.com/watch?v=CZe9D2J_psY&t=3556s, though beware that this talk was very unprepared. ↩︎
I think most types team members want to see it totally reworked or even ripped out completely… ↩︎
Well, it's because of error messages, but I was able to fix that mostly. ↩︎