Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Actually use TAIT instead of emulating it #125362

Merged
merged 2 commits into from
May 24, 2024
Merged

Conversation

joboet
Copy link
Contributor

@joboet joboet commented May 21, 2024

core's impl_fn_for_zst macro is just a hacky way of emulating TAIT. TAIT has become stable enough to be used in other places inside the standard library, so let's use it in core as well.

@rustbot
Copy link
Collaborator

rustbot commented May 21, 2024

r? @Nilstrieb

rustbot has assigned @Nilstrieb.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 21, 2024
@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@Nilstrieb Nilstrieb left a comment

Choose a reason for hiding this comment

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

lol @ the UI test, but the changes are good, so r=me after fixing the UI test (if the fix is simple)

@joboet
Copy link
Contributor Author

joboet commented May 23, 2024

It was actually quite a simple fix: The clippy lint checks that the TAIT's destructor is insignificant, but it can't/shouldn't be able to figure that out as the TAIT is opaque. Using Copy instead of Clone in the TAIT bounds implies !Drop and therefore fixes the lint.

@bors r=@Nilstrieb

@bors
Copy link
Contributor

bors commented May 23, 2024

📌 Commit c398b2c has been approved by Nilstrieb

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 23, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request May 23, 2024
…llaumeGomez

Rollup of 6 pull requests

Successful merges:

 - rust-lang#125263 (rust-lld: fallback to rustc's sysroot if there's no path to the linker in the target sysroot)
 - rust-lang#125345 (rustc_codegen_llvm: add support for writing summary bitcode)
 - rust-lang#125362 (Actually use TAIT instead of emulating it)
 - rust-lang#125412 (Don't suggest adding the unexpected cfgs to the build-script it-self)
 - rust-lang#125445 (Migrate `run-make/rustdoc-with-short-out-dir-option` to `rmake.rs`)
 - rust-lang#125452 (Cleanup check-cfg handling in core and std)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request May 24, 2024
…llaumeGomez

Rollup of 6 pull requests

Successful merges:

 - rust-lang#125263 (rust-lld: fallback to rustc's sysroot if there's no path to the linker in the target sysroot)
 - rust-lang#125345 (rustc_codegen_llvm: add support for writing summary bitcode)
 - rust-lang#125362 (Actually use TAIT instead of emulating it)
 - rust-lang#125412 (Don't suggest adding the unexpected cfgs to the build-script it-self)
 - rust-lang#125445 (Migrate `run-make/rustdoc-with-short-out-dir-option` to `rmake.rs`)
 - rust-lang#125452 (Cleanup check-cfg handling in core and std)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1e4bde1 into rust-lang:master May 24, 2024
6 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 24, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 24, 2024
Rollup merge of rust-lang#125362 - joboet:tait_hack, r=Nilstrieb

Actually use TAIT instead of emulating it

`core`'s `impl_fn_for_zst` macro is just a hacky way of emulating TAIT. TAIT has become stable enough to be used [in other places](https://github.com/rust-lang/rust/blob/e8fbd991287f637f95016a71ddc13438415bbe59/library/std/src/backtrace.rs#L431) inside the standard library, so let's use it in `core` as well.
@coolreader18
Copy link
Contributor

Is there a reason not to use TAIT for the full iterator type? e.g. type SplitAsciiWhitespaceInner<'a> = impl DoubleEndedIterator<Item = &'a str>;. Or would that interact weirdly with specialization?

oli-obk added a commit to oli-obk/rust that referenced this pull request Jun 10, 2024
…ieb"

This reverts commit 1e4bde1, reversing
changes made to 4ee97fc.
oli-obk added a commit to oli-obk/rust that referenced this pull request Jun 12, 2024
…ieb"

This reverts commit 1e4bde1, reversing
changes made to 4ee97fc.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 12, 2024
…in_sig, r=lcnr

Tait must be constrained if in sig

r? `@compiler-errors`

kind of reverts rust-lang#62423, but that PR only removed cycles in cases that we want to forbid now anyway.

see https://rust-lang.zulipchat.com/#narrow/stream/315482-t-compiler.2Fetc.2Fopaque-types/topic/lcnr.20oli.20meeting/near/370712246 for related discussion and motivating example

a TAIT showing up in a signature doesn't just mean it can register a hidden type, but that it must.

This is the [design the types team decided upon](https://hackmd.io/QOsEaEJtQK-XDS_xN4UyQA#Proposal-preferred) for the following reasons

* avoids a hypothetical situation where getting smarter in trait solving could cause new cycle errors or new inference errors to show up, where today something is treated as opaque.
* avoids having the situation where a (minor?) change to a function body causes an error, because its TAIT usage suddenly isn't "opaque enough" anymore.
* avoids having to explain why in some cases you need to put a Tait into a tiny module together with its defining usages. Now you basically gotta always do this, the moment a Tait is in a signature that isn't in the defining scope.
* avoids false-cycle errors
* anything diverging from this pattern needs to either
    * move the TAIT declaration and defining function into their own helper sub module
    * use the attributes we'll provide in the future to explicitly opt in or out of being in the defining scope

fixes rust-lang#117861

reverts rust-lang#125362 cc `@Nilstrieb` `@joboet`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants