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 10 pull requests #100593

Closed
wants to merge 27 commits into from

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

compiler-errors and others added 27 commits July 30, 2022 09:44
Having to replace - with _ (and vice versa) makes the slugs less greppable
and thus constitutes a contributor roadblock.

Result of running this repeatedly up until reaching a fixpoint:

find compiler/rustc_error_messages/locales/en-US/ -type f -exec sed -i 's/\(.+\)-\(.*\)=/\1_\2=/' {} \;

Plus some fixes to update usages of slugs leading with -.
For the most part, the macro actually worked with _ slugs, but the prefix_something -> prefix::something
conversion was not implemented.

We don't want to accept - slugs for consistency reasons.
We thus error if a name is found with - inside.
This ensures a consistent style.
Let-chaining avoids some code duplication.
`Parser::parse_bottom_expr` currently constructs an empty `attrs` and
then passes it to a large number of other functions. This makes the code
harder to read than it should be, because it's not clear that many
`attrs` arguments are always empty.

This commit removes `attrs` and the passing, simplifying a lot of
functions. The commit also renames `Parser::mk_expr` (which takes an
`attrs` argument) as `mk_expr_with_attrs`, and introduces a new
`mk_expr` which creates an expression with no attributes, which is the
more common case.
Fixes a bug where impl of items that were imported from a private module
would be striped

Fixes rust-lang#100252
Fixes rust-lang#100242
…fn-trait-error, r=cjgillot

Fix nonsense non-tupled `Fn` trait error

Given this code:

```rust
#![feature(unboxed_closures)]

fn a<F: Fn<usize>>(f: F) {}

fn main() {
    a(|_: usize| {});
}
```

We currently emit this error:
```
error[E0631]: type mismatch in closure arguments
 --> src/main.rs:6:5
  |
6 |     a(|_: usize| {});
  |     ^ ---------- found signature of `fn(usize) -> _`
  |     |
  |     expected signature of `fn(usize) -> _`
  |
note: required by a bound in `a`
 --> src/main.rs:3:9
  |
3 | fn a<F: Fn<usize>>(f: F) {}
  |         ^^^^^^^^^ required by this bound in `a`

For more information about this error, try `rustc --explain E0631`.
error: could not compile `playground` due to previous error
```
Notably, it says the same thing for "expected" and "found"!

Fix the output so that we instead emit:
```
error[E0308]: mismatched types
 --> /home/gh-compiler-errors/test.rs:6:5
  |
6 |     a(|_: usize| {});
  |     ^ types differ
  |
  = note: expected trait `Fn<usize>`
             found trait `Fn<(usize,)>`
note: required by a bound in `a`
 --> /home/gh-compiler-errors/test.rs:3:9
  |
3 | fn a<F: Fn<usize>>(f: F) {}
  |         ^^^^^^^^^ required by this bound in `a`

error: aborting due to previous error
```

The error could still use some work, namely the "mismatched types" part, but I'm leaving it a bit rough since the only way you'd ever get this error is when you're messing with `#![feature(unboxed_closures)]`.

Simply making sure we actually print out the difference in trait-refs is good enough for me. I could probably factor in some additional improvements if those are desired.
… r=michaelwoerister

improve "try ignoring the field" diagnostic

Closes rust-lang#95795
…uillaumeGomez

Rustdoc-Json: Don't remove impls for items imported from private modules

After rust-lang#99287, items in private modules may still be in the json output, if a public import accesses them. To reflect this, items that are imported need to be marked as retained in the `Stripper` pass, so their impls arn't removed by `ImplStripper`.

[More context on zulip](https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/Populating.20cache.2Eimpls), thanks to @ jyn514 for helping debug this.

`@rustbot` modify labels: +A-rustdoc-json +T-rustdoc

r? `@GuillaumeGomez`

Fixes rust-lang#100252
Fixes rust-lang#100242
Replace - with _ in fluent slugs to improve developer workflows

This is a proposal to smoothen the compiler contribution experience in the face of the move to fluent.

## Context

The fluent project has introduced a layer of abstraction to compiler errors. Previously, people would write down error messages directly in the same file the code was located to emit them. Now, there is a slug that connects the code in the compiler to the error message in the ftl file.

You can look at 7ef610c to see an example of the changes:

Old:
```Rust
let msg = format!(
    "bounds on `{}` are most likely incorrect, consider instead \
        using `{}` to detect whether a type can be trivially dropped",
    predicate,
    cx.tcx.def_path_str(needs_drop)
);
lint.build(&msg).emit();
```
New (Rust side):
```Rust
lint.build(fluent::lint::drop_trait_constraints)
    .set_arg("predicate", predicate)
    .set_arg("needs_drop", cx.tcx.def_path_str(needs_drop))
    .emit();
```
New (Fluent side):
```fluent
lint-drop-trait-constraints =
    bounds on `{$predicate}` are most likely incorrect, consider instead using `{$needs_drop}` to detect whether a type can be trivially dropped
```

You will note that in the ftl file, the slug is slightly different from the slug in the Rust file: The ftl slug uses `-` (e.g. `lint-drop-trait-constraints`) while the rust slug uses `::` and `_` (e.g. `lint::drop_trait_constraints`). This choice was probably done due to:

* Rust not accepting `-` in identifiers (as it is an operator)
* fluent not supporting the `:` character in slug names (parse error upon attempts)
* all official fluent documentation using `-` instead of `_`

## The problem

The two different types of slugs, one with `-`, and one with `_`, cause difficulties for contributors. Imagine you don't have perfect knowledge of where stuff is in the compiler (i would say this is most people), and you encounter an error for which you think there is something you could improve that is not just a rewording.

So you want to find out where in the compiler's code that error is being emitted. The best way is via grepping.

1. you grep for the message in the compiler's source code. You discover the ftl file and find out the slug for that error.
2. That slug however contains `-` instead of `_`, so you have to manually translate the `-`'s into `_`s, and furthermore either remove the leading module name, or replace the first `-` with a `::`.
3. you do a second grep to get to the emitting location in the compiler's code.

This translation difficulty in step 2 appears also in the other direction when you want to figure out what some code in the compiler is doing and use error messages to help your understanding. Comments and variable names are way less exposed to users so [are more likely going to lie](rust-lang@cc3c5d2) than error messages.

I think that at least the `-`→`_` translation which makes up most of step 2 can be removed at low cost.

## The solution

If you look closely, the practice of fluent to use `-` is only a stylistic choice and it is not enforced by fluent implementations, neither the playground nor the one the rust compiler uses, that slugs may not contain `_`. Thus, we can in fact migrate the ftl side to `_`. So now we'll have slugs like  `lint_drop_trait_constraints` on the ftl side. You only have to do one replacement now to get to the Rust slug: remove the first `_` and place a `::` in its stead. I would argue that this change is in fact useful as it allows you to control whether you want to look at the rust side of things or the ftl side of things via changing the query string only: with an increased number of translations checked into the repository, grepping for raw slugs will return the slug in many ftl files, so an explicit step to look for the source code is always useful. In the other direction (rust to fluent), you don't need a translation at all any more, as you can just take the final piece of the slug (e.g. `drop_trait_constraints`) and grep for that. The PR also adds enforcement to forbid usage of `_` in slug names. Internal slug names (those leading with a `-`) are exempt from that enforcement.

As another workflow that benefits from this change, people who add new errors don't have to do that `-` conversion either.

| Before/After | Fluent slug | Rust slug (no change) |
|--|--|--|
| Before | `lint-drop-trait-constraints` | `lint::drop_trait_constraints`|
| After | `lint_drop_trait_constraints` | `lint::drop_trait_constraints`|

Note that I've suggested this previously in the translation thread on zulip. I think it's important to think about non-translator contribution impact of fluent. I have certainly plans for more improvements, but this is a good first step.

`@rustbot` label A-diagnostics
…r=estebank

Adjust span of fn argument declaration

Span of a fn argument declaration goes from:

```
fn foo(i : i32 , ...)
       ^^^^^^^^
```

to:

```
fn foo(i : i32 , ...)
       ^^^^^^^
```

That is, we don't include the extra spacing up to the trailing comma, which I think is more correct.

cc rust-lang#99646 (comment)
r? `@estebank`

---

The two tests that had dramatic changes in their rendering I think actually are improved, though they are kinda poor spans both before and after the changes. 🤷 Thoughts?
…astorino

Delay span bug when failing to normalize negative coherence impl subject due to other malformed impls

Fixes rust-lang#100191

r? `@spastorino`
…, r=compiler-errors

Parser simplifications

Best reviewed one commit at a time.

r? `@compiler-errors`
Fix STD build for ESP-IDF

We have accidentally broken the STD build for the Tier 3 `target_os="espidf"` (only) by pushing non-buildable changes to `libc` which ended up in version 0.2.229.

`libc` [was fixed](rust-lang/libc@d0e3ff0) from V0.2.230 onwards. This PR is only upgrading the `libc` dependency in `Cargo.lock` to latest (V0.2.231).

`Cargo.lock` was modified by using `cargo update -p libc`.
…ed-enum-variant, r=notriddle

[rustdoc] Fix handling of stripped enum variant in JSON output format

Fixes rust-lang#100529.

cc `@aDotInTheVoid` `@Enselic`
r? `@notriddle`
…=jyn514

Reland changes replacing num_cpus with available_parallelism

Since rust-lang#97925 added cgroupv1 support the problem in rust-lang#97549 which lead to the previous revert should be addressed now.

Cargo has reapplied the replacement too rust-lang/cargo#10969

Reverts 1ae4b25 (part of rust-lang#97911)
Relands rust-lang#94524
@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Aug 15, 2022
@rustbot rustbot added the rollup A PR which is a rollup label Aug 15, 2022
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=10

@bors
Copy link
Contributor

bors commented Aug 15, 2022

📌 Commit cdfab3a has been approved by matthiaskrgr

It is now in the queue for this repository.

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Aug 15, 2022
@bors
Copy link
Contributor

bors commented Aug 15, 2022

⌛ Testing commit cdfab3a with merge c5e414989dd5083b02acdd124241daacd8b6292b...

@bors
Copy link
Contributor

bors commented Aug 15, 2022

💔 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 Aug 15, 2022
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
    Checking rustc_builtin_macros v0.0.0 (/checkout/compiler/rustc_builtin_macros)
error: passing `TypeError<'_>` by reference
  --> compiler/rustc_middle/src/ty/error.rs:78:29
   |
78 |     pub fn involves_regions(&self) -> bool {
   |                             ^^^^^ help: try passing by value: `TypeError<'_>`
   |
   = note: `-D rustc::pass-by-value` implied by `-D warnings`
error: could not compile `rustc_middle` due to previous error
warning: build failed, waiting for other jobs to finish...
error: could not compile `rustc_middle` due to previous error
Build completed unsuccessfully in 0:02:09

@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
    Checking rustc_builtin_macros v0.0.0 (/checkout/compiler/rustc_builtin_macros)
error: passing `TypeError<'_>` by reference
  --> compiler/rustc_middle/src/ty/error.rs:78:29
   |
78 |     pub fn involves_regions(&self) -> bool {
   |                             ^^^^^ help: try passing by value: `TypeError<'_>`
   |
   = note: `-D rustc::pass-by-value` implied by `-D warnings`
error: could not compile `rustc_middle` due to previous error
warning: build failed, waiting for other jobs to finish...
error: could not compile `rustc_middle` due to previous error
Build completed unsuccessfully in 0:02:05

@matthiaskrgr matthiaskrgr deleted the rollup-ybmxj5b branch October 9, 2022 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rollup A PR which is a rollup S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.