[pull] master from rust-lang:master #6
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![pull[bot]](https://avatars.githubusercontent.com/in/12910?s=60&v=4){kind=small}
This commit adds the headers for the top level documentation to rustdoc's existing table of contents, along with associated items. It only show two levels of headers. Going further would require the sidebar to be wider, and that seems unnecessary (the crates that have manually-built TOCs usually don't need deeply nested headers).
Inspired by discussion on #129486 this is intended to at least document the current state of the world in a more public location than throughout a series of issues.
The main `arbitrary_self_types` feature gate will shortly be reused for a new version of arbitrary self types which we are amending per [this RFC](https://github.com/rust-lang/rfcs/blob/master/text/3519-arbitrary-self-types-v2.md). The main amendments are: * _do_ support `self` types which can't safely implement `Deref` * do _not_ support generic `self` types * do _not_ support raw pointers as `self` types. This PR relates to the last of those bullet points: this strips pointer support from the current `arbitrary_self_types` feature. We expect this to cause some amount of breakage for crates using this unstable feature to allow raw pointer self types. If that's the case, we want to know about it, and we want crate authors to know of the upcoming changes. For now, this can be resolved by adding the new `arbitrary_self_types_pointers` feature to such crates. If we determine that use of raw pointers as self types is common, then we may maintain that as an unstable feature even if we come to stabilize the rest of the `arbitrary_self_types` support in future. If we don't hear that this PR is causing breakage, then perhaps we don't need it at all, even behind an unstable feature gate. [Tracking issue](#44874) This is [step 4 of the plan outlined here](#44874 (comment))
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Apparently tools like `rustfmt` require it in order to find the right `librustc_driver.so` without extra tweaks. Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
This change will also remove the current warnings in the build due to
`rustfmt` not being available (for `bindgen` output):
error: 'rustfmt' is not installed for the custom toolchain 'local'.
note: this is a custom toolchain, which cannot use `rustup component add`
help: if you built this toolchain from source, and used `rustup toolchain link`, then you may be able to build the component with `x.py`
Failed to run rustfmt: Internal rustfmt error (non-fatal, continuing)
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
This particular target does not expand into much code, so it is a good first candidate to see if we could keep this in the CI. Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
It will make it easier to add more in the future. Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
It's not needed.
It doesn't need to be in there, and the move simplifies lifetimes.
It's not necessary, and just complicates things.
…r=lcnr fix ICE when `asm_const` and `const_refs_to_static` are combined fixes #129462 fixes #126896 fixes #124164 I think this is a case that was missed in the fix for #125558, which inserts a type error in the case of an invalid (that is, non-integer) type being passed to an asm `const` operand. I'm not 100% sure that `span_mirbug_and_err` is the right macro here, but it is used earlier with `builtin_deref` and seems to do the trick. r? ``@lcnr``
clarify that addr_of creates read-only pointers Stacked Borrows does make this UB, but Tree Borrows does not. This is tied up with #56604 and other UCG discussions. Also see [this collection of links](Rust-for-Linux/linux#950 (comment)) where rustc treats `addr_of!` as a "non-mutating use". So, let's better be careful for now.
bootstrap: Try to track down why `initial_libdir` sometimes fails When I try to run `x` commands from the command-line, I occasionally see a mysterious failure that looks something like this: ```text thread 'main' panicked at src/lib.rs:341:14: called `Result::unwrap()` on an `Err` value: StripPrefixError(()) ``` It happens often enough to be annoying, but rarely enough that I can't reproduce it at will. The error message points to a particular `unwrap` call, but doesn't include enough context to determine *why* the failure occurs. Re-running the command almost always works, so I suspect some kind of filesystem race condition (possibly involving VSCode invoking bootstrap at the same time), but there's not much I can do with the information I currently have. So this PR includes some relevant information in the panic message when the failure occurs, in the hope that doing so will make the cause easier to track down when the failure occurs again.
explain why Rvalue::Len still exists I just spent a bit of time trying to remove this until I realized why that's non-trivial. Let's document that for the next person. :)
copy rustc rustlib artifacts from ci-rustc We recently (since #129311) had an issue because some rustlib files were missing (like: "error[E0463]: can't find crate for rustc_ast") when building tools that rely on rustc. This patch fixes that by copying those files as required. r? Kobzol Blocker for #122709
use the bootstrapped compiler for `test-float-parse` test Fixes #122709 (comment). Blocker for #122709
Add compat note for trait solver change r? ``@pietroalbini`` ``@BoxyUwU`` cc ``@lcnr``
…s, r=tgross35 Add digit separators in `Duration` examples ``@rustbot`` label A-docs
Temporarily remove fmease from the review rotation Namely for like a week. I seriously need to work off my review backlog! r? fmease
forward linker option to lint-docs This fixes an error found when building the doc for a cross-built toolchain. ``` warning: the code example in lint `unstable_syntax_pre_expansion` in /buildroots/chenx97/rustc-1.80.1-src/compiler/rustc_lint_defs/src/builtin.rs failed to generate the expected output: did not find lint `unstable_syntax_p re_expansion` in output of example, got: error: linking with `cc` failed: exit status: 1 ... ``` Closes: #129956
Rollup of 8 pull requests Successful merges: - #128820 (fix: get llvm type of global val) - #129028 (`impl_trait_overcaptures`: Don't worry about uncaptured contravariant lifetimes if they outlive a captured lifetime) - #129471 ([rustdoc] Sort impl associated items by kinds and then by appearance) - #129706 (Rename dump of coroutine by-move-body to be more consistent, fix ICE in dump_mir) - #129720 (Simplify DestProp memory management) - #129796 (Unify scraped examples with other code examples) - #129938 (Elaborate on deriving vs implementing `Copy`) - #129973 (run_make_support: rename `Command::stdin` to `stdin_buf` and add `std{in,out,err}` config helpers) r? `@ghost` `@rustbot` modify labels: rollup
Don't emit `expect`/`assume` in opt-level=0 LLVM does not make use of expect/assume calls in `opt-level=0`, so we can simplify IR by not emitting them in this case.
Co-authored-by: Lorenz Schmidt <bytesnake@mailbox.org>
Rollup of 11 pull requests Successful merges: - #128919 (Add an internal lint that warns when accessing untracked data) - #129472 (fix ICE when `asm_const` and `const_refs_to_static` are combined) - #129653 (clarify that addr_of creates read-only pointers) - #129775 (bootstrap: Try to track down why `initial_libdir` sometimes fails) - #129939 (explain why Rvalue::Len still exists) - #129942 (copy rustc rustlib artifacts from ci-rustc) - #129943 (use the bootstrapped compiler for `test-float-parse` test) - #129944 (Add compat note for trait solver change) - #129947 (Add digit separators in `Duration` examples) - #129955 (Temporarily remove fmease from the review rotation) - #129957 (forward linker option to lint-docs) r? `@ghost` `@rustbot` modify labels: rollup
Currently `await` is only counted towards coverage if the containing function is suspended and resumed at least once. A future commit will fix this and update the test to reflect the new behavior.
Currently `await` is only counted towards coverage if the containing function is suspended and resumed at least once. This is because it expands to code which contains a branch on the discriminant of `Poll`. By treating it like a branching macro (e.g. `assert!`), these implementation details will be hidden from the coverage results.
Check WF of source type's signature on fn pointer cast This PR patches the implied bounds holes slightly for #129005, #25860. Like most implied bounds related unsoundness fixes, this isn't complete w.r.t. higher-ranked function signatures, but I believe it implements a pretty good heuristic for now. ### What does this do? This PR makes a partial patch for a soundness hole in a `FnDef` -> `FnPtr` "reifying" pointer cast where we were never checking that the signature we are casting *from* is actually well-formed. Because of this, and because `FnDef` doesn't require its signature to be well-formed (just its predicates must hold), we are essentially allowed to "cast away" implied bounds that are assumed within the body of the `FnDef`: ``` fn foo<'a, 'b, T>(_: &'a &'b (), v: &'b T) -> &'a T { v } fn bad<'short, T>(x: &'short T) -> &'static T { let f: fn(_, &'short T) -> &'static T = foo; f(&&(), x) } ``` In this example, subtyping ends up casting the `_` type (which should be `&'static &'short ()`) to some other type that no longer serves as a "witness" to the lifetime relationship `'short: 'static` which would otherwise be required for this call to be WF. This happens regardless of if `foo`'s lifetimes are early- or late-bound. This PR implements two checks: 1. We check that the signature of the `FnDef` is well-formed *before* casting it. This ensures that there is at least one point in the MIR where we ensure that the `FnDef`'s implied bounds are actually satisfied by the caller. 2. Implements a special case where if we're casting from a higher-ranked `FnDef` to a non-higher-ranked, we instantiate the binder of the `FnDef` with *infer vars* and ensure that it is a supertype of the target of the cast. The (2.) is necessary to validate that these pointer casts are valid for higher-ranked `FnDef`. Otherwise, the example above would still pass even if `help`'s `'a` lifetime were late-bound. ### Further work The WF checks for function calls are scattered all over the MIR. We check the WF of args in call terminators, we check the WF of `FnDef` when we create a `const` operand referencing it, and we check the WF of the return type in #115538, to name a few. One way to make this a bit cleaner is to simply extend #115538 to always check that the signature is WF for `FnDef` types. I may do this as a follow-up, but I wanted to keep this simple since this leads to some pretty bad NLL diagnostics regressions, and AFAICT this solution is *complete enough*. ### Crater triage Done here: #129021 (comment) r? lcnr
…larsan68 Make `./x.py <cmd> compiler/<crate>` aware of the crate's features Does not fix #129727 on its own as the way the parallel-compiler cfg and feature flags are setup being generally incompatible with `resolver = 2` but it progresses on the issue. But this should in theory allow compiler crates to work that do not depend on the parallel compiler stuff (so some leaf crates).
… r=workingjubilee
Inaccurate `{Path,OsStr}::to_string_lossy()` documentation
The documentation of `Path::to_string_lossy()` and `OsStr::to_string_lossy()` says the following:
> Any non-Unicode sequences are replaced with `U+FFFD REPLACEMENT CHARACTER`
which didn't immediately make sense to me. ("non-Unicode sequences"?)
Since both `to_string_lossy` functions eventually become just a call to `String::from_utf8_lossy`, I believe the documentation meant to say:
> Any *non-UTF-8* sequences are replaced with `U+FFFD REPLACEMENT CHARACTER`
This PR corrects this mistake in the documentation.
For the record, a similar quote can be found in the documentation of `String::from_utf8_lossy`:
> ... During this conversion, `from_utf8_lossy()` will replace any invalid UTF-8 sequences with `U+FFFD REPLACEMENT CHARACTER`, ...
Make `Ty::boxed_ty` return an `Option` Looks like a good place to use Rust's type system. --- Most of https://github.com/rust-lang/rust/blob/4ac7bcbaad8d6fd7a51bdf1b696cbc3ba4c796cf/compiler/rustc_middle/src/ty/sty.rs#L971-L1963 looks like it could be moved to `TyKind` (then I guess `Ty` should be made to deref to `TyKind`).
…otes, r=pietroalbini Remove wasm32-wasip2's tier 2 status from release notes It turns out the stars did not actually align for this to get released in Rust 1.81 alas. Full tier 2 status for `wasm32-wasip2` required two PRs: * #126967 - this made it into Rust 1.81 * #127867 - this didn't make the cut and is in Rust 1.82 instead This wasn't caught until just after today's release so the plan is to remove the release notes for 1.81 and coordinate to instead add these as release notes to 1.82.
coverage: Count await when the Future is immediately ready Currently `await` is only counted towards coverage if the containing function is suspended and resumed at least once. This is because it expands to code which contains a branch on the discriminant of `Poll`. By treating it like a branching macro (e.g. `assert!`), these implementation details will be hidden from the coverage results. I added a test to ensure the fix works in simple cases, but the heuristic of picking only the first await-related covspan might be unreliable. I plan on testing more thoroughly with a real codebase over the next couple of weeks. closes #98712
Rollup of 6 pull requests Successful merges: - #129021 (Check WF of source type's signature on fn pointer cast) - #129781 (Make `./x.py <cmd> compiler/<crate>` aware of the crate's features) - #129963 (Inaccurate `{Path,OsStr}::to_string_lossy()` documentation) - #129969 (Make `Ty::boxed_ty` return an `Option`) - #129995 (Remove wasm32-wasip2's tier 2 status from release notes) - #130013 (coverage: Count await when the Future is immediately ready ) r? `@ghost` `@rustbot` modify labels: rollup
Autodiff Upstreaming - enzyme backend Tracking issue: #124509 Part of #129175 This PR should allow building Enzyme from source on Tier 1 targets (when also building LLVM), except MSVC. It's only a small fraction (~200 lines) of the whole upstream PR, but due to bootstrapping and the number of configurations in which rustc can be build I assume that this will be the hardest to merge, so I'm starting with it. Happy to hear what changes are required to be able to upstream this code. **Content:** It contains a new configure flag `--enable-llvm-enzyme`, and will build the new Enzyme submodule when it is set. **Discussion:** Apparently Rust CI isn't able to clone repositories outside the rust-lang org? At least I'm seeing this error in CI: ``` git@github.com: Permission denied (publickey). fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. ``` Does that mean we would need to mirror github.com/EnzymeAD/Enzyme in rust-lang, until LLVM upgrades Enzyme from an Incubator project to something that ships as part of the monorepo? Tracking: - #124509
stabilize `-Znext-solver=coherence` r? `@compiler-errors` --- This PR stabilizes the use of the next generation trait solver in coherence checking by enabling `-Znext-solver=coherence` by default. More specifically its use in the *implicit negative overlap check*. The tracking issue for this is #114862. Closes #114862. ## Background ### The next generation trait solver The new solver lives in [`rustc_trait_selection::solve`](https://github.com/rust-lang/rust/blob/master/compiler/rustc_trait_selection/src/solve/mod.rs) and is intended to replace the existing *evaluate*, *fulfill*, and *project* implementation. It also has a wider impact on the rest of the type system, for example by changing our approach to handling associated types. For a more detailed explanation of the new trait solver, see the [rustc-dev-guide](https://rustc-dev-guide.rust-lang.org/solve/trait-solving.html). This does not stabilize the current behavior of the new trait solver, only the behavior impacting the implicit negative overlap check. There are many areas in the new solver which are not yet finalized. We are confident that their final design will not conflict with the user-facing behavior observable via coherence. More on that further down. Please check out [the chapter](https://rustc-dev-guide.rust-lang.org/solve/significant-changes.html) summarizing the most significant changes between the existing and new implementations. ### Coherence and the implicit negative overlap check Coherence checking detects any overlapping impls. Overlapping trait impls always error while overlapping inherent impls result in an error if they have methods with the same name. Coherence also results in an error if any other impls could exist, even if they are currently unknown. This affects impls which may get added to upstream crates in a backwards compatible way and impls from downstream crates. Coherence failing to detect overlap is generally considered to be unsound, even if it is difficult to actually get runtime UB this way. It is quite easy to get ICEs due to bugs in coherence. It currently consists of two checks: The [orphan check] validates that impls do not overlap with other impls we do not know about: either because they may be defined in a sibling crate, or because an upstream crate is allowed to add it without being considered a breaking change. The [overlap check] validates that impls do not overlap with other impls we know about. This is done as follows: - Instantiate the generic parameters of both impls with inference variables - Equate the `TraitRef`s of both impls. If it fails there is no overlap. - [implicit negative]: Check whether any of the instantiated `where`-bounds of one of the impls definitely do not hold when using the constraints from the previous step. If a `where`-bound does not hold, there is no overlap. - *explicit negative (still unstable, ignored going forward)*: Check whether the any negated `where`-bounds can be proven, e.g. a `&mut u32: Clone` bound definitely does not hold as an explicit `impl<T> !Clone for &mut T` exists. The overlap check has to *prove that unifying the impls does not succeed*. This means that **incorrectly getting a type error during coherence is unsound** as it would allow impls to overlap: coherence has to be *complete*. Completeness means that we never incorrectly error. This means that during coherence we must only add inference constraints if they are definitely necessary. During ordinary type checking [this does not hold](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=01d93b592bd9036ac96071cbf1d624a9), so the trait solver has to behave differently, depending on whether we're in coherence or not. The implicit negative check only considers goals to "definitely not hold" if they could not be implemented downstream, by a sibling, or upstream in a backwards compatible way. If the goal is is "unknowable" as it may get added in another crate, we add an ambiguous candidate: [source](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/solve/assembly/mod.rs#L858-L883). [orphan check]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L566-L579 [overlap check]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L92-L98 [implicit negative]: https://github.com/rust-lang/rust/blob/fd80c02c168c2dfbb82c29d2617f524d2723205b/compiler/rustc_trait_selection/src/traits/coherence.rs#L223-L281 ## Motivation Replacing the existing solver in coherence fixes soundness bugs by removing sources of incompleteness in the type system. The new solver separately strengthens coherence, resulting in more impls being disjoint and passing the coherence check. The concrete changes will be elaborated further down. We believe the stabilization to reduce the likelihood of future bugs in coherence as the new implementation is easier to understand and reason about. It allows us to remove the support for coherence and implicit-negative reasoning in the old solver, allowing us to remove some code and simplifying the old trait solver. We will only remove the old solver support once this stabilization has reached stable to make sure we're able to quickly revert in case any unexpected issues are detected before then. Stabilizing the use of the next-generation trait solver expresses our confidence that its current behavior is intended and our work towards enabling its use everywhere will not require any breaking changes to the areas used by coherence checking. We are also confident that we will be able to replace the existing solver everywhere, as maintaining two separate systems adds a significant maintainance burden. ## User-facing impact and reasoning ### Breakage due to improved handling of associated types The new solver fixes multiple issues related to associated types. As these issues caused coherence to consider more types distinct, fixing them results in more overlap errors. This is therefore a breaking change. #### Structurally relating aliases containing bound vars Fixes #102048. In the existing solver relating ambiguous projections containing bound variables is structural. This is *incomplete* and allows overlapping impls. These was mostly not exploitable as the same issue also caused impls to not apply when trying to use them. The new solver defers alias-relating to a nested goal, fixing this issue: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Trait {} trait Project { type Assoc<'a>; } impl Project for u32 { type Assoc<'a> = &'a u32; } // Eagerly normalizing `<?infer as Project>::Assoc<'a>` is ambiguous, // so the old solver ended up structurally relating // // (?infer, for<'a> fn(<?infer as Project>::Assoc<'a>)) // // with // // ((u32, fn(&'a u32))) // // Equating `&'a u32` with `<u32 as Project>::Assoc<'a>` failed, even // though these types are equal modulo normalization. impl<T: Project> Trait for (T, for<'a> fn(<T as Project>::Assoc<'a>)) {} impl<'a> Trait for (u32, fn(&'a u32)) {} //[next]~^ ERROR conflicting implementations of trait `Trait` for type `(u32, for<'a> fn(&'a u32))` ``` A crater run did not discover any breakage due to this change. #### Unknowable candidates for higher ranked trait goals This avoids an unsoundness by attempting to normalize in `trait_ref_is_knowable`, fixing #114061. This is a side-effect of supporting lazy normalization, as that forces us to attempt to normalize when checking whether a `TraitRef` is knowable: [source](https://github.com/rust-lang/rust/blob/47dd709bedda8127e8daec33327e0a9d0cdae845/compiler/rustc_trait_selection/src/solve/assembly/mod.rs#L754-L764). ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait IsUnit {} impl IsUnit for () {} pub trait WithAssoc<'a> { type Assoc; } // We considered `for<'a> <T as WithAssoc<'a>>::Assoc: IsUnit` // to be knowable, even though the projection is ambiguous. pub trait Trait {} impl<T> Trait for T where T: 'static, for<'a> T: WithAssoc<'a>, for<'a> <T as WithAssoc<'a>>::Assoc: IsUnit, { } impl<T> Trait for Box<T> {} //[next]~^ ERROR conflicting implementations of trait `Trait` ``` The two impls of `Trait` overlap given the following downstream crate: ```rust use dep::*; struct Local; impl WithAssoc<'_> for Box<Local> { type Assoc = (); } ``` There a similar coherence unsoundness caused by our handling of aliases which is fixed separately in #117164. This change breaks the [`derive-visitor`](https://crates.io/crates/derive-visitor) crate. I have opened an issue in that repo: nikis05/derive-visitor#16. ### Evaluating goals to a fixpoint and applying inference constraints In the old implementation of the implicit-negative check, each obligation is [checked separately without applying its inference constraints](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/traits/coherence.rs#L323-L338). The new solver instead [uses a `FulfillmentCtxt`](https://github.com/rust-lang/rust/blob/bea5bebf3defc56e5e3446b4a95c685dbb885fd3/compiler/rustc_trait_selection/src/traits/coherence.rs#L315-L321) for this, which evaluates all obligations in a loop until there's no further inference progress. This is necessary for backwards compatibility as we do not eagerly normalize with the new solver, resulting in constraints from normalization to only get applied by evaluating a separate obligation. This also allows more code to compile: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Mirror { type Assoc; } impl<T> Mirror for T { type Assoc = T; } trait Foo {} trait Bar {} // The self type starts out as `?0` but is constrained to `()` // due to the where-clause below. Because `(): Bar` is known to // not hold, we can prove the impls disjoint. impl<T> Foo for T where (): Mirror<Assoc = T> {} //[current]~^ ERROR conflicting implementations of trait `Foo` for type `()` impl<T> Foo for T where T: Bar {} fn main() {} ``` The old solver does not run nested goals to a fixpoint in evaluation. The new solver does do so, strengthening inference and improving the overlap check: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait Foo {} impl<T> Foo for (u8, T, T) {} trait NotU8 {} trait Bar {} impl<T, U: NotU8> Bar for (T, T, U) {} trait NeedsFixpoint {} impl<T: Foo + Bar> NeedsFixpoint for T {} impl NeedsFixpoint for (u8, u8, u8) {} trait Overlap {} impl<T: NeedsFixpoint> Overlap for T {} impl<T, U: NotU8, V> Overlap for (T, U, V) {} //[current]~^ ERROR conflicting implementations of trait `Foo` ``` ### Breakage due to removal of incomplete candidate preference Fixes #107887. In the old solver we incompletely prefer the builtin trait object impl over user defined impls. This can break inference guidance, inferring `?x` in `dyn Trait<u32>: Trait<?x>` to `u32`, even if an explicit impl of `Trait<u64>` also exists. This caused coherence to incorrectly allow overlapping impls, resulting in ICEs and a theoretical unsoundness. See #107887 (comment). This compiles on stable but results in an overlap error with `-Znext-solver=coherence`: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence struct W<T: ?Sized>(*const T); trait Trait<T: ?Sized> { type Assoc; } // This would trigger the check for overlap between automatic and custom impl. // They actually don't overlap so an impl like this should remain possible // forever. // // impl Trait<u64> for dyn Trait<u32> {} trait Indirect {} impl Indirect for dyn Trait<u32, Assoc = ()> {} impl<T: Indirect + ?Sized> Trait<u64> for T { type Assoc = (); } // Incomplete impl where `dyn Trait<u32>: Trait<_>` does not hold, but // `dyn Trait<u32>: Trait<u64>` does. trait EvaluateHack<U: ?Sized> {} impl<T: ?Sized, U: ?Sized> EvaluateHack<W<U>> for T where T: Trait<U, Assoc = ()>, // incompletely constrains `_` to `u32` U: IsU64, T: Trait<U, Assoc = ()>, // incompletely constrains `_` to `u32` { } trait IsU64 {} impl IsU64 for u64 {} trait Overlap<U: ?Sized> { type Assoc: Default; } impl<T: ?Sized + EvaluateHack<W<U>>, U: ?Sized> Overlap<U> for T { type Assoc = Box<u32>; } impl<U: ?Sized> Overlap<U> for dyn Trait<u32, Assoc = ()> { //[next]~^ ERROR conflicting implementations of trait `Overlap<_>` type Assoc = usize; } ``` ### Considering region outlives bounds in the `leak_check` For details on the `leak_check`, see the FCP proposal in #119820.[^leak_check] [^leak_check]: which should get moved to the dev-guide once that PR lands :3 In both coherence and during candidate selection, the `leak_check` relies on the region constraints added in `evaluate`. It therefore currently does not register outlives obligations: [source](https://github.com/rust-lang/rust/blob/ccb1415eac3289b5ebf64691c0190dc52e0e3d0e/compiler/rustc_trait_selection/src/traits/select/mod.rs#L792-L810). This was likely done as a performance optimization without considering its impact on the `leak_check`. This is the case as in the old solver, *evaluatation* and *fulfillment* are split, with evaluation being responsible for candidate selection and fulfillment actually registering all the constraints. This split does not exist with the new solver. The `leak_check` can therefore eagerly detect errors caused by region outlives obligations. This improves both coherence itself and candidate selection: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence trait LeakErr<'a, 'b> {} // Using this impl adds an `'b: 'a` bound which results // in a higher-ranked region error. This bound has been // previously ignored but is now considered. impl<'a, 'b: 'a> LeakErr<'a, 'b> for () {} trait NoOverlapDir<'a> {} impl<'a, T: for<'b> LeakErr<'a, 'b>> NoOverlapDir<'a> for T {} impl<'a> NoOverlapDir<'a> for () {} //[current]~^ ERROR conflicting implementations of trait `NoOverlapDir<'_>` // -------------------------------------- // necessary to avoid coherence unknowable candidates struct W<T>(T); trait GuidesSelection<'a, U> {} impl<'a, T: for<'b> LeakErr<'a, 'b>> GuidesSelection<'a, W<u32>> for T {} impl<'a, T> GuidesSelection<'a, W<u8>> for T {} trait NotImplementedByU8 {} trait NoOverlapInd<'a, U> {} impl<'a, T: GuidesSelection<'a, W<U>>, U> NoOverlapInd<'a, U> for T {} impl<'a, U: NotImplementedByU8> NoOverlapInd<'a, U> for () {} //[current]~^ conflicting implementations of trait `NoOverlapInd<'_, _>` ``` ### Removal of `fn match_fresh_trait_refs` The old solver tries to [eagerly detect unbounded recursion](https://github.com/rust-lang/rust/blob/b14fd2359f47fb9a14bbfe55359db4bb3af11861/compiler/rustc_trait_selection/src/traits/select/mod.rs#L1196-L1211), forcing the affected goals to be ambiguous. This check is only an approximation and has not been added to the new solver. The check is not necessary in the new solver and it would be problematic for caching. As it depends on all goals currently on the stack, using a global cache entry would have to always make sure that doing so does not circumvent this check. This changes some goals to error - or succeed - instead of failing with ambiguity. This allows more code to compile: ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence // Need to use this local wrapper for the impls to be fully // knowable as unknowable candidate result in ambiguity. struct Local<T>(T); trait Trait<U> {} // This impl does not hold, but is ambiguous in the old // solver due to its overflow approximation. impl<U> Trait<U> for Local<u32> where Local<u16>: Trait<U> {} // This impl holds. impl Trait<Local<()>> for Local<u8> {} // In the old solver, `Local<?t>: Trait<Local<?u>>` is ambiguous, // resulting in `Local<?u>: NoImpl`, also being ambiguous. // // In the new solver the first impl does not apply, constraining // `?u` to `Local<()>`, causing `Local<()>: NoImpl` to error. trait Indirect<T> {} impl<T, U> Indirect<U> for T where T: Trait<U>, U: NoImpl {} // Not implemented for `Local<()>` trait NoImpl {} impl NoImpl for Local<u8> {} impl NoImpl for Local<u16> {} // `Local<?t>: Indirect<Local<?u>>` cannot hold, so // these impls do not overlap. trait NoOverlap<U> {} impl<T: Indirect<U>, U> NoOverlap<U> for T {} impl<T, U> NoOverlap<Local<U>> for Local<T> {} //~^ ERROR conflicting implementations of trait `NoOverlap<Local<_>>` ``` ### Non-fatal overflow The old solver immediately emits a fatal error when hitting the recursion limit. The new solver instead returns overflow. This both allows more code to compile and is results in performance and potential future compatability issues. Non-fatal overflow is generally desirable. With fatal overflow, changing the order in which we evaluate nested goals easily causes breakage if we have goal which errors and one which overflows. It is also required to prevent breakage due to the removal of `fn match_fresh_trait_refs`, e.g. [in `typenum`](rust-lang/trait-system-refactor-initiative#73). #### Enabling more code to compile In the below example, the old solver first tried to prove an overflowing goal, resulting in a fatal error. The new solver instead returns ambiguity due to overflow for that goal, causing the implicit negative overlap check to succeed as `Box<u32>: NotImplemented` does not hold. ```rust // revisions: current next //[next] compile-flags: -Znext-solver=coherence //[current] ERROR overflow evaluating the requirement trait Indirect<T> {} impl<T: Overflow<()>> Indirect<T> for () {} trait Overflow<U> {} impl<T, U> Overflow<U> for Box<T> where U: Indirect<Box<Box<T>>>, {} trait NotImplemented {} trait Trait<U> {} impl<T, U> Trait<U> for T where // T: NotImplemented, // causes old solver to succeed U: Indirect<T>, T: NotImplemented, {} impl Trait<()> for Box<u32> {} ``` #### Avoiding hangs with non-fatal overflow Simply returning ambiguity when reaching the recursion limit can very easily result in hangs, e.g. ```rust trait Recur {} impl<T, U> Recur for ((T, U), (U, T)) where (T, U): Recur, (U, T): Recur, {} trait NotImplemented {} impl<T: NotImplemented> Recur for T {} ``` This can happen quite frequently as it's easy to have exponential blowup due to multiple nested goals at each step. As the trait solver is depth-first, this immediately caused a fatal overflow error in the old solver. In the new solver we have to handle the whole proof tree instead, which can very easily hang. To avoid this we restrict the recursion depth after hitting the recursion limit for the first time. We also **ignore all inference constraints from goals resulting in overflow**. This is mostly backwards compatible as any overflow in the old solver resulted in a fatal error. ### sidenote about normalization We return ambiguous nested goals of `NormalizesTo` goals to the caller and ignore their impact when computing the `Certainty` of the current goal. See the [normalization chapter](https://rustc-dev-guide.rust-lang.org/solve/normalization.html) for more details.This means we apply constraints resulting from other nested goals and from equating the impl header when normalizing, even if a nested goal results in overflow. This is necessary to avoid breaking the following example: ```rust trait Trait { type Assoc; } struct W<T: ?Sized>(*mut T); impl<T: ?Sized> Trait for W<W<T>> where W<T>: Trait, { type Assoc = (); } // `W<?t>: Trait<Assoc = u32>` does not hold as // `Assoc` gets normalized to `()`. However, proving // the where-bounds of the impl results in overflow. // // For this to continue to compile we must not discard // constraints from normalizing associated types. trait NoOverlap {} impl<T: Trait<Assoc = u32>> NoOverlap for T {} impl<T: ?Sized> NoOverlap for W<T> {} ``` #### Future compatability concerns Non-fatal overflow results in some unfortunate future compatability concerns. Changing the approach to avoid more hangs by more strongly penalizing overflow can cause breakage as we either drop constraints or ignore candidates necessary to successfully compile. Weakening the overflow penalities instead allows more code to compile and strengthens inference while potentially causing more code to hang. While the current approach is not perfect, we believe it to be good enough. We believe it to apply the necessary inference constraints to avoid breakage and expect there to not be any desirable patterns broken by our current penalities. Similarly we believe the current constraints to avoid most accidental hangs. Ignoring constraints of overflowing goals is especially useful, as it may allow major future optimizations to our overflow handling. See [this summary](https://hackmd.io/ATf4hN0NRY-w2LIVgeFsVg) and the linked documents in case you want to know more. ### changes to performance In general, trait solving during coherence checking is not significant for performance. Enabling the next-generation trait solver in coherence does not impact our compile time benchmarks. We are still unable to compile the benchmark suite when fully enabling the new trait solver. There are rare cases where the new solver has significantly worse performance due to non-fatal overflow, its reliance on fixpoint algorithms and the removal of the `fn match_fresh_trait_refs` approximation. We encountered such issues in [`typenum`](https://crates.io/crates/typenum) and believe it should be [pretty much as bad as it can get](rust-lang/trait-system-refactor-initiative#73). Due to an improved structure and far better caching, we believe that there is a lot of room for improvement and that the new solver will outperform the existing implementation in nearly all cases, sometimes significantly. We have not yet spent any time micro-optimizing the implementation and have many unimplemented major improvements, such as fast-paths for trivial goals. TODO: get some rough results here and put them in a table ### Unstable features #### Unsupported unstable features The new solver currently does not support all unstable features, most notably `#![feature(generic_const_exprs)]`, `#![feature(associated_const_equality)]` and `#![feature(adt_const_params)]` are not yet fully supported in the new solver. We are confident that supporting them is possible, but did not consider this to be a priority. This stabilization introduces new ICE when using these features in impl headers. #### fixes to `#![feature(specialization)]` - fixes #105782 - fixes #118987 #### fixes to `#![feature(type_alias_impl_trait)]` - fixes #119272 - #105787 (comment) - fixes #124207 ## This does not stabilize the whole solver While this stabilizes the use of the new solver in coherence checking, there are many parts of the solver which will remain fully unstable. We may still adapt these areas while working towards stabilizing the new solver everywhere. We are confident that we are able to do so without negatively impacting coherence. ### goals with a non-empty `ParamEnv` Coherence always uses an empty environment. We therefore do not depend on the behavior of `AliasBound` and `ParamEnv` candidates. We only stabilizes the behavior of user-defined and builtin implementations of traits. There are still many open questions there. ### opaque types in the defining scope The handling of opaque types - `impl Trait` - in both the new and old solver is still not fully figured out. Luckily this can be ignored for now. While opaque types are reachable during coherence checking by using `impl_trait_in_associated_types`, the behavior during coherence is separate and self-contained. The old and new solver fully agree here. ### normalization is hard This stabilizes that we equate associated types involving bound variables using deferred-alias-equality. We also stop eagerly normalizing in coherence, which should not have any user-facing impact. We do not stabilize the normalization behavior outside of coherence, e.g. we currently deeply normalize all types during writeback with the new solver. This may change going forward ### how to replace `select` from the old solver We sometimes depend on getting a single `impl` for a given trait bound, e.g. when resolving a concrete method for codegen/CTFE. We do not depend on this during coherence, so the exact approach here can still be freely changed going forward. ## Acknowledgements This work would not have been possible without `@compiler-errors.` He implemented large chunks of the solver himself but also and did a lot of testing and experimentation, eagerly discovering multiple issues which had a significant impact on our approach. `@BoxyUwU` has also done some amazing work on the solver. Thank you for the endless hours of discussion resulting in the current approach. Especially the way aliases are handled has gone through multiple revisions to get to its current state. There were also many contributions from - and discussions with - other members of the community and the rest of `@rust-lang/types.` This solver builds upon previous improvements to the compiler, as well as lessons learned from `chalk` and `a-mir-formality`. Getting to this point would not have been possible without that and I am incredibly thankful to everyone involved. See the [list of relevant PRs](https://github.com/rust-lang/rust/pulls?q=is%3Apr+is%3Amerged+label%3AWG-trait-system-refactor+-label%3Arollup+closed%3A%3C2024-03-22+).
Use `DeepRejectCtxt` to quickly reject `ParamEnv` candidates The description is on the [zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/144729-t-types/topic/.5Basking.20for.20help.5D.20.60DeepRejectCtxt.60.20for.20param.20env.20candidates) r? `@lcnr`
pull bot
pushed a commit
that referenced
this pull request
Oct 17, 2024
…raheemdev
Optimize `Box::default` and `Arc::default` to construct more types in place
Both the `Arc` and `Box` `Default` impls currently call `T::default()` before allocating, and then moving the resulting `T` into the allocation.
Most `Default` impls are trivial, which should in theory allow
LLVM to construct `T: Default` directly in the `Box` allocation when calling
`<Box<T>>::default()`.
However, the allocation may fail, which necessitates calling `T`'s destructor if it has one.
If the destructor is non-trivial, then LLVM has a hard time proving that it's
sound to elide, which makes it construct `T` on the stack first, and then copy it into the allocation.
Change both of these impls to allocate first, and then call `T::default` into the uninitialized allocation, so that LLVM doesn't have to prove that it's sound to elide the destructor/initial stack copy.
For example, given the following Rust code:
```rust
#[derive(Default, Clone)]
struct Foo {
x: Vec<u8>,
z: String,
y: Vec<u8>,
}
#[no_mangle]
pub fn src() -> Box<Foo> {
Box::default()
}
```
<details open>
<summary>Before this PR:</summary>
```llvm
`@__rust_no_alloc_shim_is_unstable` = external global i8
; drop_in_place() generated in case the allocation fails
; core::ptr::drop_in_place<playground::Foo>
; Function Attrs: nounwind nonlazybind uwtable
define internal fastcc void `@"_ZN4core3ptr36drop_in_place$LT$playground..Foo$GT$17hff376aece491233bE"(ptr` noalias nocapture noundef readonly align 8 dereferenceable(72) %_1) unnamed_addr #0 personality ptr `@rust_eh_personality` {
start:
%_1.val = load i64, ptr %_1, align 8
%0 = icmp eq i64 %_1.val, 0
br i1 %0, label %bb6, label %"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i"
"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i": ; preds = %start
%1 = getelementptr inbounds i8, ptr %_1, i64 8
%_1.val6 = load ptr, ptr %1, align 8, !nonnull !3, !noundef !3
tail call void `@__rust_dealloc(ptr` noundef nonnull %_1.val6, i64 noundef %_1.val, i64 noundef 1) #8
br label %bb6
bb6: ; preds = %"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i", %start
%2 = getelementptr inbounds i8, ptr %_1, i64 24
%.val9 = load i64, ptr %2, align 8
%3 = icmp eq i64 %.val9, 0
br i1 %3, label %bb5, label %"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i.i11"
"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i.i11": ; preds = %bb6
%4 = getelementptr inbounds i8, ptr %_1, i64 32
%.val10 = load ptr, ptr %4, align 8, !nonnull !3, !noundef !3
tail call void `@__rust_dealloc(ptr` noundef nonnull %.val10, i64 noundef %.val9, i64 noundef 1) #8
br label %bb5
bb5: ; preds = %"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i.i11", %bb6
%5 = getelementptr inbounds i8, ptr %_1, i64 48
%.val4 = load i64, ptr %5, align 8
%6 = icmp eq i64 %.val4, 0
br i1 %6, label %"_ZN4core3ptr46drop_in_place$LT$alloc..vec..Vec$LT$u8$GT$$GT$17hb5ca95423e113cf7E.exit16", label %"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i15"
"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i15": ; preds = %bb5
%7 = getelementptr inbounds i8, ptr %_1, i64 56
%.val5 = load ptr, ptr %7, align 8, !nonnull !3, !noundef !3
tail call void `@__rust_dealloc(ptr` noundef nonnull %.val5, i64 noundef %.val4, i64 noundef 1) #8
br label %"_ZN4core3ptr46drop_in_place$LT$alloc..vec..Vec$LT$u8$GT$$GT$17hb5ca95423e113cf7E.exit16"
"_ZN4core3ptr46drop_in_place$LT$alloc..vec..Vec$LT$u8$GT$$GT$17hb5ca95423e113cf7E.exit16": ; preds = %bb5, %"_ZN63_$LT$alloc..alloc..Global$u20$as$u20$core..alloc..Allocator$GT$10deallocate17heaa87468709346b1E.exit.i.i.i4.i15"
ret void
}
; Function Attrs: nonlazybind uwtable
define noalias noundef nonnull align 8 ptr `@src()` unnamed_addr #1 personality ptr `@rust_eh_personality` {
start:
; alloca to place `Foo` in.
%_1 = alloca [72 x i8], align 8
call void `@llvm.lifetime.start.p0(i64` 72, ptr nonnull %_1)
store i64 0, ptr %_1, align 8
%_2.sroa.4.0._1.sroa_idx = getelementptr inbounds i8, ptr %_1, i64 8
store ptr inttoptr (i64 1 to ptr), ptr %_2.sroa.4.0._1.sroa_idx, align 8
%_2.sroa.5.0._1.sroa_idx = getelementptr inbounds i8, ptr %_1, i64 16
%_3.sroa.4.0..sroa_idx = getelementptr inbounds i8, ptr %_1, i64 32
call void `@llvm.memset.p0.i64(ptr` noundef nonnull align 8 dereferenceable(16) %_2.sroa.5.0._1.sroa_idx, i8 0, i64 16, i1 false)
store ptr inttoptr (i64 1 to ptr), ptr %_3.sroa.4.0..sroa_idx, align 8
%_3.sroa.5.0..sroa_idx = getelementptr inbounds i8, ptr %_1, i64 40
%_4.sroa.4.0..sroa_idx = getelementptr inbounds i8, ptr %_1, i64 56
call void `@llvm.memset.p0.i64(ptr` noundef nonnull align 8 dereferenceable(16) %_3.sroa.5.0..sroa_idx, i8 0, i64 16, i1 false)
store ptr inttoptr (i64 1 to ptr), ptr %_4.sroa.4.0..sroa_idx, align 8
%_4.sroa.5.0..sroa_idx = getelementptr inbounds i8, ptr %_1, i64 64
store i64 0, ptr %_4.sroa.5.0..sroa_idx, align 8
%0 = load volatile i8, ptr `@__rust_no_alloc_shim_is_unstable,` align 1, !noalias !4
%_0.i.i.i = tail call noalias noundef align 8 dereferenceable_or_null(72) ptr `@__rust_alloc(i64` noundef 72, i64 noundef 8) #8, !noalias !4
%1 = icmp eq ptr %_0.i.i.i, null
br i1 %1, label %bb2.i, label %"_ZN5alloc5boxed12Box$LT$T$GT$3new17h0864de14f863a27aE.exit"
bb2.i: ; preds = %start
; invoke alloc::alloc::handle_alloc_error
invoke void `@_ZN5alloc5alloc18handle_alloc_error17h98142d0d8d74161bE(i64` noundef 8, i64 noundef 72) #9
to label %.noexc unwind label %cleanup.i
.noexc: ; preds = %bb2.i
unreachable
cleanup.i: ; preds = %bb2.i
%2 = landingpad { ptr, i32 }
cleanup
; call core::ptr::drop_in_place<playground::Foo>
call fastcc void `@"_ZN4core3ptr36drop_in_place$LT$playground..Foo$GT$17hff376aece491233bE"(ptr` noalias noundef nonnull align 8 dereferenceable(72) %_1) #10
resume { ptr, i32 } %2
"_ZN5alloc5boxed12Box$LT$T$GT$3new17h0864de14f863a27aE.exit": ; preds = %start
; Copy from stack to heap if allocation is successful
call void `@llvm.memcpy.p0.p0.i64(ptr` noundef nonnull align 8 dereferenceable(72) %_0.i.i.i, ptr noundef nonnull align 8 dereferenceable(72) %_1, i64 72, i1 false)
call void `@llvm.lifetime.end.p0(i64` 72, ptr nonnull %_1)
ret ptr %_0.i.i.i
}
```
</details>
<details>
<summary>After this PR</summary>
```llvm
; Notice how there's no `drop_in_place()` generated as well
define noalias noundef nonnull align 8 ptr `@src()` unnamed_addr #0 personality ptr `@rust_eh_personality` {
start:
; no stack allocation
%0 = load volatile i8, ptr `@__rust_no_alloc_shim_is_unstable,` align 1
%_0.i.i.i.i.i = tail call noalias noundef align 8 dereferenceable_or_null(72) ptr `@__rust_alloc(i64` noundef 72, i64 noundef 8) #5
%1 = icmp eq ptr %_0.i.i.i.i.i, null
br i1 %1, label %bb3.i, label %"_ZN5alloc5boxed16Box$LT$T$C$A$GT$13new_uninit_in17h80d6355ef4b73ea3E.exit"
bb3.i: ; preds = %start
; call alloc::alloc::handle_alloc_error
tail call void `@_ZN5alloc5alloc18handle_alloc_error17h98142d0d8d74161bE(i64` noundef 8, i64 noundef 72) #6
unreachable
"_ZN5alloc5boxed16Box$LT$T$C$A$GT$13new_uninit_in17h80d6355ef4b73ea3E.exit": ; preds = %start
; construct `Foo` directly into the allocation if successful
store i64 0, ptr %_0.i.i.i.i.i, align 8
%_8.sroa.4.0._1.sroa_idx = getelementptr inbounds i8, ptr %_0.i.i.i.i.i, i64 8
store ptr inttoptr (i64 1 to ptr), ptr %_8.sroa.4.0._1.sroa_idx, align 8
%_8.sroa.5.0._1.sroa_idx = getelementptr inbounds i8, ptr %_0.i.i.i.i.i, i64 16
%_8.sroa.7.0._1.sroa_idx = getelementptr inbounds i8, ptr %_0.i.i.i.i.i, i64 32
tail call void `@llvm.memset.p0.i64(ptr` noundef nonnull align 8 dereferenceable(16) %_8.sroa.5.0._1.sroa_idx, i8 0, i64 16, i1 false)
store ptr inttoptr (i64 1 to ptr), ptr %_8.sroa.7.0._1.sroa_idx, align 8
%_8.sroa.8.0._1.sroa_idx = getelementptr inbounds i8, ptr %_0.i.i.i.i.i, i64 40
%_8.sroa.10.0._1.sroa_idx = getelementptr inbounds i8, ptr %_0.i.i.i.i.i, i64 56
tail call void `@llvm.memset.p0.i64(ptr` noundef nonnull align 8 dereferenceable(16) %_8.sroa.8.0._1.sroa_idx, i8 0, i64 16, i1 false)
store ptr inttoptr (i64 1 to ptr), ptr %_8.sroa.10.0._1.sroa_idx, align 8
%_8.sroa.11.0._1.sroa_idx = getelementptr inbounds i8, ptr %_0.i.i.i.i.i, i64 64
store i64 0, ptr %_8.sroa.11.0._1.sroa_idx, align 8
ret ptr %_0.i.i.i.i.i
}
```
</details>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot]
Can you help keep this open source service alive? 💖 Please sponsor : )