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

Rollup of 8 pull requests #123762

Merged
merged 19 commits into from
Apr 11, 2024
Merged

Rollup of 8 pull requests #123762

merged 19 commits into from
Apr 11, 2024

Conversation

fmease
Copy link
Member

@fmease fmease commented Apr 10, 2024

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

adamgemmell and others added 19 commits April 2, 2024 12:05
This PR aims to pin down exactly what restricted_std is meant to achieve
and what it isn't.

This commit fixes rust-lang/wg-cargo-std-aware#87
by explaining why the error appears and what the choices the user has.
The error describes how std cannot function without knowing about some
form of OS/platform support. Any features of std that work without an
OS should be moved to core/alloc (see rust-lang#27242
rust-lang#103765).

Note that the message says "platform" and "environment" because, since
rust-lang#120232, libstd can be built for
some JSON targets. This is still unsupported (all JSON targets probably
should be unstable rust-lang/wg-cargo-std-aware#90),
but a JSON target with the right configuration should hopefully have
some partial libstd support.

I propose closing rust-lang/wg-cargo-std-aware#69
as "Won't fix" since any support of std without properly configured os,
vendor or env fields is very fragile considering future upgrades of Rust
or dependencies. In addition there's no likely path to it being fixed
long term (making std buildable for all targets being the only
solution). This is distinct from tier 3 platforms with limited std
support implemented (and as such aren't restricted_std) because these
platforms can conceptually work in the future and std support should
mainly improve over time.

The alternative to closing rust-lang/wg-cargo-std-aware#69
is a new crate feature for std which escapes the restricted_std
mechanism in build.rs. It could be used with the -Zbuild-std-features
flag if we keep it permanently unstable, which I hope we can do anyway.
A minor side-effect in this scenario is that std wouldn't be marked as
unstable if documentation for it were generated with build-std.
Newly stable API:

```rust
impl CStr {
    pub fn count_bytes(&self) -> usize;
}
```

Const stabilization has not yet been decided, so that will continue to be
gated under <rust-lang#113219>.

Fixes: <rust-lang#114441>
When suggesting a type on inference error, do not use `{closure@..}`.
Instead, replace with an appropriate `fn` ptr.

On the error message, use `short_ty_string` and write long types to
disk.

```
error[E0284]: type annotations needed for `Select<{[email protected]:2782:13}, _, Expression<'_>, _>`
  --> crates/lang/src/parser.rs:41:13
   |
41 |         let lit = select! {
   |             ^^^
42 |             Token::Int(i) = e => Expression::new(Expr::Lit(ast::Lit::Int(i.parse().unwrap())), e.span()),
   |                                                                                                  ---- type must be known at this point
   |
   = note: the full type name has been written to '/home/gh-estebank/iowo/target/debug/deps/lang-e2d6e25819442273.long-type-4587393693885174369.txt'
   = note: cannot satisfy `<_ as chumsky::input::Input<'_>>::Span == SimpleSpan`
help: consider giving `lit` an explicit type, where the type for type parameter `I` is specified
   |
41 |         let lit: Select<for<'a, 'b> fn(tokens::Token<'_>, &'a mut MapExtra<'_, 'b, _, _>) -> Option<Expression<'_>>, _, Expression<'_>, _> = select! {
   |                +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
```

instead of

```
error[E0284]: type annotations needed for `Select<{closure@/home/gh-estebank/.cargo/registry/src/index.crates.io-6f17d22bba15001f/chumsky-1.0.0-alpha.6/src/lib.rs:2782:13: 2782:28}, _, Expression<'_>, _>`
  --> crates/lang/src/parser.rs:41:13
   |
41 |         let lit = select! {
   |             ^^^
42 |             Token::Int(i) = e => Expression::new(Expr::Lit(ast::Lit::Int(i.parse().unwrap())), e.span()),
   |                                                                                                  ---- type must be known at this point
   |
   = note: cannot satisfy `<_ as chumsky::input::Input<'_>>::Span == SimpleSpan`
help: consider giving `lit` an explicit type, where the type for type parameter `I` is specified
   |
41 |         let lit: Select<{closure@/home/gh-estebank/.cargo/registry/src/index.crates.io-6f17d22bba15001f/chumsky-1.0.0-alpha.6/src/lib.rs:2782:13: 2782:28}, _, Expression<'_>, _> = select! {
   |                ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
```

Fix rust-lang#123630.
This reverts commit 049a917.

The resolution to <rust-lang#123282> is
that the `f16`/`f128` regression in the beta compiler was fixable
without a revert, so the commit adding `#[cfg(not(bootstrap))]` is no
longer useful (added in
<rust-lang#123390>).

Revert this commit because not having these basic impls bootstrap-gated
simplifies everything else that uses them.
Implement basic operation traits that get lowered to intrinsics. This
includes codegen tests for implemented operations.
Create empty modules so `rustdoc` has someplace to link to for these
types.
…r=Amanieu

`f16` and `f128` step 4: basic library support

This is the next step after rust-lang#121926, another portion of rust-lang#114607

Tracking issue: rust-lang#116909

This PR adds the most basic operations to `f16` and `f128` that get lowered as LLVM intrinsics. This is a very small step but it seemed reasonable enough to add unopinionated basic operations before the larger modules that are built on top of them.

r? ```@Amanieu``` since you were pretty involved in the RFC
cc ```@compiler-errors```
```@rustbot``` label +T-libs-api +S-blocked +F-f16_and_f128
…r=petrochenkov

Be more specific when flagging imports as redundant due to the extern prelude

There are multiple distinct kinds of [preludes](https://doc.rust-lang.org/reference/names/preludes.html). Be more specific when flagging imports as redundant due to the [extern prelude](https://doc.rust-lang.org/reference/names/preludes.html#extern-prelude).

r? Nilstrieb or compiler
Skip `unused_parens` report for `Paren(Path(..))` in macro.

fixes rust-lang#120642

In following code, `unused_parens` suggest change `<($($rest),*)>::bar()` to `<$rest>::bar()`  which will cause another err: `error: variable 'rest' is still repeating at this depth`:

```rust
trait Foo {
    fn bar();
}

macro_rules! problem {
    ($ty:ident) => {
        impl<$ty: Foo> Foo for ($ty,) {
            fn bar() { <$ty>::bar() }
        }
    };
    ($ty:ident $(, $rest:ident)*) => {
        impl<$ty: Foo, $($rest: Foo),*> Foo for ($ty, $($rest),*) {
            fn bar() {
                <$ty>::bar();
                <($($rest),*)>::bar()
            }
        }
        problem!($($rest),*);
    }
}
```

I think maybe we can handle this by avoid warning for `Paren(Path(..))` in the macro. Is this reasonable approach?
…d-std, r=ehuss

Document restricted_std

This PR aims to pin down exactly what restricted_std is meant to achieve and what it isn't.

This commit fixes rust-lang/wg-cargo-std-aware#87 by explaining why the error appears and what the choices the user has. The error describes how std cannot function without knowing about some form of OS/platform support. Any features of std that work without an OS should be moved to core/alloc (see rust-lang#27242 rust-lang#103765).

Note that the message says "platform" and "environment" because, since rust-lang#120232, libstd can be built for some JSON targets. This is still unsupported (all JSON targets probably should be unstable rust-lang/wg-cargo-std-aware#90), but a JSON target with the right configuration should hopefully have some partial libstd support.

I propose closing rust-lang/wg-cargo-std-aware#69 as "Won't fix" since any support of std without properly configured os, vendor or env fields is very fragile considering future upgrades of Rust or dependencies. In addition there's no likely path to it being fixed long term (making std buildable for all targets being the only solution). This is distinct from tier 3 platforms with limited std support implemented (and as such aren't restricted_std) because these platforms can conceptually work in the future and std support should mainly improve over time.

The alternative to closing rust-lang/wg-cargo-std-aware#69 is a new crate feature for std which escapes the restricted_std mechanism in build.rs. It could be used with the -Zbuild-std-features flag if we keep it permanently unstable, which I hope we can do anyway. A minor side-effect in this scenario is that std wouldn't be marked as unstable if documentation for it were generated with build-std.

cc ```@ehuss```
…s, r=dtolnay

Stabilize `cstr_count_bytes`

Newly stable API:

```rust
impl CStr {
    pub fn count_bytes(&self) -> usize;
}
```

Const stabilization has not yet been decided, so that will continue to be gated under <rust-lang#113219>.

FCP finished at rust-lang#114441 (comment).

Fixes: <rust-lang#114441>
Use `fn` ptr signature instead of `{closure@..}` in infer error

When suggesting a type on inference error, do not use `{closure@..}`. Instead, replace with an appropriate `fn` ptr.

On the error message, use `short_ty_string` and write long types to disk.

```
error[E0284]: type annotations needed for `Select<{[email protected]:2782:13}, _, Expression<'_>, _>`
  --> crates/lang/src/parser.rs:41:13
   |
41 |         let lit = select! {
   |             ^^^
42 |             Token::Int(i) = e => Expression::new(Expr::Lit(ast::Lit::Int(i.parse().unwrap())), e.span()),
   |                                                                                                  ---- type must be known at this point
   |
   = note: the full type name has been written to '/home/gh-estebank/iowo/target/debug/deps/lang-e2d6e25819442273.long-type-4587393693885174369.txt'
   = note: cannot satisfy `<_ as chumsky::input::Input<'_>>::Span == SimpleSpan`
help: consider giving `lit` an explicit type, where the type for type parameter `I` is specified
   |
41 |         let lit: Select<for<'a, 'b> fn(tokens::Token<'_>, &'a mut MapExtra<'_, 'b, _, _>) -> Option<Expression<'_>>, _, Expression<'_>, _> = select! {
   |                +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
```

instead of

```
error[E0284]: type annotations needed for `Select<{closure@/home/gh-estebank/.cargo/registry/src/index.crates.io-6f17d22bba15001f/chumsky-1.0.0-alpha.6/src/lib.rs:2782:13: 2782:28}, _, Expression<'_>, _>`
  --> crates/lang/src/parser.rs:41:13
   |
41 |         let lit = select! {
   |             ^^^
42 |             Token::Int(i) = e => Expression::new(Expr::Lit(ast::Lit::Int(i.parse().unwrap())), e.span()),
   |                                                                                                  ---- type must be known at this point
   |
   = note: cannot satisfy `<_ as chumsky::input::Input<'_>>::Span == SimpleSpan`
help: consider giving `lit` an explicit type, where the type for type parameter `I` is specified
   |
41 |         let lit: Select<{closure@/home/gh-estebank/.cargo/registry/src/index.crates.io-6f17d22bba15001f/chumsky-1.0.0-alpha.6/src/lib.rs:2782:13: 2782:28}, _, Expression<'_>, _> = select! {
   |                ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
```

Address rust-lang#123630 (test missing).
clean up docs for `File::sync_*`

* Clarify that `sync_all` also writes data and not just metadata.
* Clarify that dropping a file is not equivalent to calling `sync_all` and ignoring the result. `sync_all` the still the recommended way to detect errors before closing, because we don't have a dedicated method for that.
* Add a link from `sync_all` to `sync_data`, because that's what the user might want to use instead.
* Add doc aliases for `fsync` -> `sync_all` and `fdatasync` -> `sync_data`. Those are the POSIX standard names for these functions. I was trying to find out what we call `fsync` in Rust and had to search through the source code to find it, so this alias should help with that in the future.
…trait, r=estebank

Use `suggest_impl_trait` in return type suggestion on type error

Discovered while doing other refactoring. Review with whitespace disabled.

r? estebank
@rustbot 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Apr 10, 2024
@fmease
Copy link
Member Author

fmease commented Apr 10, 2024

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented Apr 10, 2024

📌 Commit 2930dce has been approved by fmease

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 Apr 10, 2024
@bors
Copy link
Contributor

bors commented Apr 11, 2024

⌛ Testing commit 2930dce with merge 3ea4a4e...

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 11, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#122470 (`f16` and `f128` step 4: basic library support)
 - rust-lang#122954 (Be more specific when flagging imports as redundant due to the extern prelude)
 - rust-lang#123314 (Skip `unused_parens` report for `Paren(Path(..))` in macro.)
 - rust-lang#123360 (Document restricted_std)
 - rust-lang#123661 (Stabilize `cstr_count_bytes`)
 - rust-lang#123703 (Use `fn` ptr signature instead of `{closure@..}` in infer error)
 - rust-lang#123756 (clean up docs for `File::sync_*`)
 - rust-lang#123761 (Use `suggest_impl_trait` in return type suggestion on type error)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Apr 11, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 11, 2024
@rust-log-analyzer
Copy link
Collaborator

The job dist-aarch64-apple failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@compiler-errors
Copy link
Member

@bors retry

@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 Apr 11, 2024
@bors
Copy link
Contributor

bors commented Apr 11, 2024

⌛ Testing commit 2930dce with merge 4435924...

@bors
Copy link
Contributor

bors commented Apr 11, 2024

☀️ Test successful - checks-actions
Approved by: fmease
Pushing 4435924 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 11, 2024
@bors bors merged commit 4435924 into rust-lang:master Apr 11, 2024
12 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Apr 11, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#122470 f16 and f128 step 4: basic library support 707d6562f55b55a7aab4ab3a904997116b358314 (link)
#122954 Be more specific when flagging imports as redundant due to … 38cba6c7973f1df5fe0619c1ebc2cd1b38a8919e (link)
#123314 Skip unused_parens report for Paren(Path(..)) in macro. 1ecac1c12a9b70dc9a8b1584fb9580856a59c015 (link)
#123360 Document restricted_std 2a57ecea569ca0c81a503088315297eeb3d2fde3 (link)
#123661 Stabilize cstr_count_bytes cffb07d720542fcf76d7b0c1e114b34f3aa032d6 (link)
#123703 Use fn ptr signature instead of {closure@..} in infer e… ab58d1f775d0284f0b9fc5e2b81125d5a40a7ef0 (link)
#123756 clean up docs for File::sync_* e9947ec523ccc4e0c70401b3c28ae6f01327c43e (link)
#123761 Use suggest_impl_trait in return type suggestion on type … 2845af096a6e129455b40de4378566f7c9173bf9 (link)

previous master: aa067fb984

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (4435924): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

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.4% [0.2%, 0.5%] 2
Regressions ❌
(secondary)
0.5% [0.3%, 1.7%] 9
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.8% [-4.2%, -1.4%] 2
All ❌✅ (primary) 0.4% [0.2%, 0.5%] 2

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)
-3.4% [-3.4%, -3.4%] 1
Improvements ✅
(secondary)
-3.8% [-6.1%, -2.7%] 5
All ❌✅ (primary) -3.4% [-3.4%, -3.4%] 1

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)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-4.3% [-5.3%, -3.4%] 2
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 675.211s -> 675.519s (0.05%)
Artifact size: 315.98 MiB -> 315.99 MiB (0.00%)

@rustbot rustbot added the perf-regression Performance regression. label Apr 11, 2024
@fmease fmease deleted the rollup-jdfx54z branch April 11, 2024 06:56
@Kobzol
Copy link
Contributor

Kobzol commented Apr 16, 2024

The coercions regression is noise, the rest were small regressions in doc builds, I don't think that further investigation is needed.

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. 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.