Rollup of 6 pull requests - #161668
Closed
JonathanBrouwer wants to merge 14 commits into
Closed
Conversation
so that we can specify more than one i32 of padding.
This renaming should at least make Session and Builder easier to distinguish.
The meaning of `TokenTreeCursor::index` is context-dependent: in the innermost (current) `TokenTreeCursor` it points to the next token tree, but in all the other (stack) `TokenTreeCursor`s it points to the current token tree. This makes the meanings of "current", "next", and "look_ahead" confusing for it and for `TokenCursor`. This commit clarifies things by adjusting the stack `TokenTreeCursor`s to also point to the next token tree, and by improving various comments. The commit also renames `TokenCursor::next` as `TokenCursor::next_and_bump` for consistency with everything else: `next` means "get the next thing" and `bump` means "advance the cursor", and this operation does both.
make `pad_i32` of `PassMode::cast` an integer
so that we can specify more than one i32 of padding. This PR only adds the functionality but does not yet use it: there should be no functional changes.
This is needed for the ABI of `Complex<{ float }>` on 32-bit powerpc. Other mechanisms, e.g. using `PassMode::prefixed` don't appear to work.
More discussion is in [#t-compiler/help > power complex abi](https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/power.20complex.20abi/with/613294420).
…JonathanBrouwer Remove `From<!> for T` *reservation* impl This PR removes the `<T> From<!> for T` *reservation* implementation added in rust-lang#62661 and tracked in rust-lang#64715 and rust-lang#64631. The reservation impl in question was added in order to reserve some space for adding the following impl: ```rust impl<T> From<!> for T { fn from(never: !) -> T { never } } ``` It is meant to prevent users from writing *some* impls that would overlap if the `From<!> for T` impl is to be added. This requires T-types FCP. Below is my proposal and necessary context: ## The reservation impl is not sufficient The reservation impl prevents one from assuming that `From<!> for T` is not implemented making the following [not compile](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2024&gist=220e375c77db9a88c360230283f888cb): ```rust struct LocalType; trait SomeTrait { } impl<T: From<!>> SomeTrait for T { } impl SomeTrait for LocalType { } ``` However, it does not prevent *all* implementation that would overlap given `From<!> for T`. Namely, `From<!> for T` would overlap with the following impls, all of which are currently permitted (and exist): ```rust // T for T identity impl in `core` impl<T> From<T> for T { ... } // Various T->wrapper of T impls present in both the standard library, // and in external crates impl<T> From<T> for W<T> { ... } // !->Local is also allowed impl From<!> for Local {} ``` Also note that the reservation impl only exists for `From<!>`, but not for `From<Infallible>`, so even the impls that the reservation impl is meant to forbid, are currently allowed through `Infallible` anyway (we are planning to make `Infallible` a type alias to `!` at the same time as stabilizing `!`). ## Motivation for `From<!> for T` impl It is surprisingly hard to find the original motivation for `From<!> for T` impl or the reservation impl, other than "people vaguely think that all types should implement `From<!>`, since there is never-to-any coercion". One use-case seems to be "calling infallible function in a fallible one, and unwrapping `Result<_, !>` with `?`". However, nowdays it is trivial to unwrap the result safely without `?`: ```rust let Ok(owo) = infallible_function(); ``` Another use-case that I've seen [mentioned](rust-lang#62661 (comment)) is "fallible function with a set error, taking an infallible function": ```rust fn try_from<T>(t: T) -> Result<Meow, MyError> where Meow: TryFrom<T>, <Meow as TryFrom<T>>::Error: Into<MyError> { ... } ``` With such definition, you can't pass `Meow` into `try_from`, because `MyError: From<Infallible>` doesn't hold. This is more unfortunate, but it's not clear how widespread this problem is and how bad the workarounds would be. If a function expects `impl FnOnce(...) -> Result<...>`, it should be trivial to coerce the `!` error to an appropriate type. With other trait bounds (like in the example above) it could be solved by adding a custom impl for your specific error type (annoying, but workable). Certaintly this doesn't feel like a big roadblock to me. (let me know if you know more prior art on this) ## There is no clear path for adding `From<!> for T` impl Adding `From<!> for T` seems... hard... and hard to argue for. It would require ignoring overlap with a *bunch* of impls (as described above) and would also require low priority impls (to avoid inference failures in cases where previously the only applicable impl was the identity one, so adding `From<!>` makes "one impl rule" not apply). The [tracking issue](rust-lang#64715) says: > The precise mechanism to permit us to add the `From<!> for T` impl is not yet clear. The current "plan of record" is to extend the ["marker trait mechanism"](rust-lang#29864) to accommodate the idea of impls whose entire body consists of unreachable methods and to permit overlap. Considering "traits with all methods having arguments of uninhabited types" as marker traits is technically possible (I think?), but feels like a bit of a stretch. Making overlap check consider if all trait functions take arguments which are uninhabited (known to be uninhabited *in the current context*) seems like a big complication, especially considering how `From<T>` would not be a marker trait in the general case — only `From<!>`/`From<OtherUninhabitedTypes>` would be (also that requires attaching the overlap check to some context from which we can check if a type is publically uninhabited, which can also lead to situations where impl `A` overlaps with impl `B`, but impl `B` doesn't overlap with impl `A`[^1]). Allowing overlap with arbitrary user impls also is likely to cause unforcene issues in my opinion. [^1]: i.e. in the context of impl `A` a certain type is not known to be uninhabited, and thus the overlap between impls should not be allowed. at the same time in the context of impl `B` same type might be known to be uninhabited, allowing the overlap. ## The reservation impl causes problems for the never type stabilization Because the reservation impl reserves space for `From<!> for T`, but not for `From<Infallible> for T`, making `Infallible` an alias for `!` makes some code fail to compile. See rust-lang#155924: > 3. standard library contains a [reservation impl](https://doc.rust-lang.org/1.94.0/src/core/convert/mod.rs.html#802-806), which forbids [certain](rust-lang#64715) `From<!>` impls. After making `Infallible = !`, this reservation impl can conflict with existing implementations for `Infallible` - This breaks 14 crates total (including reverse-dependencies of broken crates) Given that both keeping the reservation impl (while making `Infallible = !`) and making the reservation a proper impl break code, we should decide which path we want to pursue before stabilizing the never type (and making `Infallible = !`). ## Proposal After trying to add `From<!> for T` as a proper impl, I'm not convinced that it's worth the complexity and messiness of allowing such widespread overlap (with user defined impls too!). **As such, I propose to remove the reservation impl**, to prevent unnecessary breakage from its combination with making `Infallible = !`, as described above. ## Alternatives - Add proper `impl<T> From<!> for T`, accepting the breakage, overlap, and the complexity. - I'm not sure how feasible this is, after trying to do this approach, it doesn't feel right - Keep the reservation impl / add the reservation impl `From<Infallible> for T` formally accepting the breakage of that, with the hopes that we can still add `impl<T> From<!> for T` in the future - This is the most breaking of the option, as it breaks code that depends on `From<Infallible> for T` not existing, *and* expects future breakage when adding `impl<T> From<!> for T` - It is unlikely that adding `impl<T> From<!> for T` in the future will be much easier than right now ----- Closes rust-lang#64715 r? types I'll remove the `rustc_reservation_impl` attribute in a separate PR (cc rust-lang#64631).
bootstrap: Rename `Build` to `Session` - Follow-up to rust-lang#161277 --- Having separate types named `Build` and `Builder` is quite confusing, especially since *build* by itself isn't a very informative name in the context of a build system. This PR therefore performs the change suggested in rust-lang#161277 (comment), renaming `Build` to `Session`. While *session* is not exactly the most descriptive name either, it at least has the virtue of being clearly distinct from *builder*. That should be helpful when trying to clarify the different roles of the two types. There should be no change to bootstrap behaviour.
…s, r=petrochenkov delegation: add tests for delegations to inherent impls Second part of rust-lang#160505, for more convenient reviewing. Part of rust-lang#118212. r? @petrochenkov
…r=Kobzol Clarify token cursor behaviour The meaning of `TokenTreeCursor::index` is context-dependent: in the innermost (current) `TokenTreeCursor` it points to the next token tree, but in all the other (stack) `TokenTreeCursor`s it points to the current token tree. This makes the meanings of "current", "next", and "look_ahead" confusing for it and for `TokenCursor`. This commit clarifies things by adjusting the stack `TokenTreeCursor`s to also point to the next token tree, and by improving various comments. The commit also renames `TokenCursor::next` as `TokenCursor::next_and_bump` for consistency with everything else: `next` means "get the next thing" and `bump` means "advance the cursor", and this operation does both. r? @Kobzol
…Brouwer Add codegen test for Vec::clear lowering to an unconditional store Closes rust-lang#45459
rustbot
added
A-LLVM
Member
Author
|
@bors r+ p=5 |
Contributor
rust-bors
Bot
added
S-waiting-on-bors
rust-bors
Bot
added
S-waiting-on-author
Contributor
|
This pull request was unapproved due to being closed. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
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.
Successful merges:
pad_i32ofPassMode::castan integer #160132 (makepad_i32ofPassMode::castan integer)From<!> for T*reservation* impl #160705 (RemoveFrom<!> for Treservation impl)BuildtoSession#161600 (bootstrap: RenameBuildtoSession)r? @ghost
Create a similar rollup