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 4 pull requests #91078

Closed
wants to merge 11 commits into from

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

Noratrieb and others added 11 commits November 15, 2021 20:40
This also fixes the same suggestion, which was kind of broken, because it just searched for the last occurence of `const` to replace with a `let`. This works great in some cases, but when there is no const and a leading space to the file, it doesn't work and panic with overflow because it thought that it had found a const.

I also changed the suggestion to only trigger if the `const` and the non-constant value are on the same line, because if they aren't, the suggestion is very likely to be wrong.

Also don't trigger the suggestion if the found `const` is on line 0, because that triggers the ICE.
…ewline

I cannot provide a test for that thanks to tidy.
…aused-by-reexporting-pub-crate-visibility-to-outside, r=oli-obk

Clarify error messages caused by re-exporting `pub(crate)` visibility to outside

This PR clarifies error messages and suggestions caused by re-exporting pub(crate) visibility outside the crate.

Here is a small example ([Rust Playground](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=e2cd0bd4422d4f20e6522dcbad167d3b)):

```rust
mod m {
    pub(crate) enum E {}
}
pub use m::E;

fn main() {}
```

This code is compiled to:

```
error[E0365]: `E` is private, and cannot be re-exported
 --> prog.rs:4:9
  |
4 | pub use m::E;
  |         ^^^^ re-export of private `E`
  |
  = note: consider declaring type or module `E` with `pub`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0365`.
```

However, enum `E` is actually public to the crate, not private totally—nevertheless, rustc treats `pub(crate)` and private visibility as the same on the error messages. They are not clear and should be segmented distinctly.

By applying changes in this PR, the error message below will be the following message that would be clearer:

```
error[E0365]: `E` is only public to inside of the crate, and cannot be re-exported outside
 --> prog.rs:4:9
  |
4 | pub use m::E;
  |         ^^^^ re-export of crate public `E`
  |
  = note: consider declaring type or module `E` with `pub`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0365`.
```
…r=estebank

Fix `non-constant value` ICE (rust-lang#90878)

This also fixes the same suggestion, which was kind of broken, because it just searched for the last occurence of `const` to replace with a `let`. This works great in some cases, but when there is no const and a leading space to the file, it doesn't work and panic with overflow because it thought that it had found a const.

I also changed the suggestion to only trigger if the `const` and the non-constant value are on the same line, because if they aren't, the suggestion is very likely to be wrong.

Also don't trigger the suggestion if the found `const` is on line 0, because that triggers the ICE.

Asking Esteban to review since he was the last one to change the relevant code.

r? `@estebank`

Fixes rust-lang#90878
…=jsha

Make scrollbar in the sidebar always visible for visual consistency

Fixes rust-lang#90943.

I had to add a background in `dark` and `ayu` themes, otherwise it was looking strange (like an invisible margin). So it looks like this:

![Screenshot from 2021-11-17 14-45-49](https://user-images.githubusercontent.com/3050060/142212476-18892ae0-ba4b-48e3-8c0f-4ca1dd2f851d.png)
![Screenshot from 2021-11-17 14-45-53](https://user-images.githubusercontent.com/3050060/142212482-e97b2fad-68d2-439a-b62e-b56e6ded5345.png)

Sadly, I wasn't able to add a GUI test to ensure that the scrollbar was always displayed because it seems not possible in puppeteer for whatever reason... I used this method: on small pages (like `lib2/sub_mod/index.html`), comparing `.navbar`'s `clientWidth` with `offsetWidth` (the first doesn't include the sidebar in the computed amount). When checking in the browser, it works fine but in puppeteer it almost never works...

In case anyone want to try to solve the bug, here is the puppeteer code:

<details>
More information about this: I tried another approach which was to get the element in `evaluate` directly (by calling it from `page.evaluate(() => { .. });` directly instead of `parseAssertElemProp.evaluate(e => {...});`.

```js
const puppeteer = require('puppeteer');

(async () => {
    const browser = await puppeteer.launch();
    const page = await browser.newPage();
    await page.goto("file:///path/rust/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/doc/lib2/sub_mod/index.html");
    await page.waitFor(".sidebar");
    let parseAssertElemProp = await page.$(".sidebar");
    if (parseAssertElemProp === null) { throw '".sidebar" not found'; }
    await parseAssertElemProp.evaluate(e => {
        const parseAssertElemPropDict = {"clientWidth": "192", "offsetWidth":"200"};
        for (const [parseAssertElemPropKey, parseAssertElemPropValue] of Object.entries(parseAssertElemPropDict)) {
            if (e[parseAssertElemPropKey] === undefined || String(e[parseAssertElemPropKey]) != parseAssertElemPropValue) {
                throw 'expected `' + parseAssertElemPropValue + '` for property `' + parseAssertElemPropKey + '` for selector `.sidebar`, found `' + e[parseAssertElemPropKey] + '`';
            }
        }
    }).catch(e => console.error(e));
    await browser.close();
})();
```

</details>

r? `@jsha`
…, r=estebank

Elaborate `Future::Output` when printing opaque `impl Future` type

I would love to see the `Output =` type when printing type errors involving opaque `impl Future`.

[Test code](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=a800b481edd31575fbcaf5771a9c3678)

Before (cut relevant part of output):
```
note: while checking the return type of the `async fn`
 --> /home/michael/test.rs:5:19
  |
5 | async fn bar() -> usize {
  |                   ^^^^^ checked the `Output` of this `async fn`, found opaque type
  = note:     expected type `usize`
          found opaque type `impl Future`
```

After:
```
note: while checking the return type of the `async fn`
 --> /home/michael/test.rs:5:19
  |
5 | async fn bar() -> usize {
  |                   ^^^^^ checked the `Output` of this `async fn`, found opaque type
  = note:     expected type `usize`
          found opaque type `impl Future<Output = usize>`
```

Note the "found opaque type `impl Future<Output = usize>`" in the new output.

----

Questions:
1. We skip printing the output type when it's a projection, since I have been seeing some types like `impl Future<Output = <[static generator@/home/michael/test.rs:2:11: 2:21] as Generator<ResumeTy>>::Return>` which are not particularly helpful and leak implementation detail.
    * Am I able to normalize this type within `rustc_middle::ty::print::pretty`? Alternatively, can we normalize it when creating the diagnostic? Otherwise, I'm fine with skipping it and falling back to the old output.
    * Should I suppress any other types? I didn't encounter anything other than this generator projection type.
2. Not sure what the formatting of this should be. Do I include spaces in `Output = `?
@rustbot rustbot added the rollup A PR which is a rollup label Nov 20, 2021
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=4

@bors
Copy link
Contributor

bors commented Nov 20, 2021

📌 Commit 1e446ef has been approved by matthiaskrgr

@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 Nov 20, 2021
@matthiaskrgr matthiaskrgr deleted the rollup-hxrixcs branch November 20, 2021 15:04
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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants