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 21 pull requests #74408

Merged
merged 90 commits into from
Jul 16, 2020
Merged

Rollup of 21 pull requests #74408

merged 90 commits into from
Jul 16, 2020

Conversation

Manishearth
Copy link
Member

Successful merges:

Failed merges:

r? @ghost

alexcrichton and others added 30 commits June 26, 2020 09:23
Currently dependency crates of the standard library can sometimes leak
into error messages such as when traits to import are suggested.
Additionally they can leak into documentation such as in the list of
"all traits implemented by `u32`". The dependencies of the standard
library, however, are intended to be private.

The dependencies of the standard library can't actually be stabl-y
imported nor is the documentation that relevant since you can't import
them on stable either. This commit updates both the compiler and rustdoc
to ignore unstable traits in these two scenarios.

Specifically the suggestion for traits to import ignore unstable traits,
and similarly the list of traits implemented by a type excludes unstable
traits.

This commit is extracted from rust-lang#73441 where the addition of some new
dependencies to the standard library was showed to leak into various
error messages and documentation. The intention here is to go ahead and
land these changes ahead of that since it will likely take some time to
land.
This commit also categorizing access as Field, Index, or Subslice.

Ideas are taken from `mir::ProjectionElem`.

Proposed changes: https://github.com/rust-lang/project-rfc-2229/blob/master/hir-place-target.md

Closes: rust-lang/project-rfc-2229#1,
rust-lang/project-rfc-2229#2

Co-authored-by: Aman Arora <[email protected]>
Co-authored-by: Chris Pardy <[email protected]>
Co-authored-by: Dhruv Jauhar <[email protected]>
Fixes rust-lang#72684.

This commit also modifies the signature of `Classifier::new` to avoid
copying the source being highlighted.
to avoid surprising listed in API guidelines.
Instead, ignore resolution errors that occur in item bodies.

The reason this can't ignore item bodies altogether is because
`const fn` could be used in generic types, for example `[T; f()]`
- Explicitly check for missing docs
- Don't run any lints except those we explicitly specified
- Remove unnecessary `should_loop` variable
- Report errors for trait implementations

These should give resolution errors because they are visible outside the
current scope. Without these errors, rustdoc will give ICEs:

```
thread 'rustc' panicked at 'attempted .def_id() on invalid res: Err', /home/joshua/src/rust/src/libstd/macros.rs:16:9
  15: rustc_hir::def::Res<Id>::def_id
             at /home/joshua/src/rust/src/librustc_hir/def.rs:382
  16: rustdoc::clean::utils::register_res
             at src/librustdoc/clean/utils.rs:627
  17: rustdoc::clean::utils::resolve_type
             at src/librustdoc/clean/utils.rs:587
```

- Add much more extensive tests
  + fn -> impl -> fn
  + fn -> impl -> fn -> macro
  + errors in function parameters
  + errors in trait bounds
  + errors in the type implementing the trait
  + unknown bounds for the type
  + unknown types in function bodies
  + errors generated by macros

- Use explicit state instead of trying to reconstruct it from random info
- Use an enum instead of a boolean
- Add example of ignored error
Instead, report the error.

This emits the errors on-demand, without special-casing `impl Trait`, so
it should catch all ICEs of this kind, including ones that haven't been
found yet.

Since the error is emitted during type-checking there is less info about
the error; see comments in the code for details.

- Add test case for -> impl Trait
- Add test for impl trait with alias
- Move EmitIgnoredResolutionErrors to rustdoc

This makes `fn typeck_item_bodies` public, which is not desired behavior.
That change should be removed once
rust-lang#74070 is merged.

- Don't visit nested closures twice
Previously, rustdoc would issue a delay_span_bug ICE on the following code:

```rust
pub fn a() -> impl Fn() -> u32 {
    || content::doesnt::matter()
}
```

This wasn't picked up earlier because having `type Alias = impl Trait;`
in the same module caused _all closures_ to be typechecked, even if they
wouldn't normally. Additionally, if _any_ error was emitted, no
delay_span_bug would be emitted. So as part of this commit all of the
tests were separated out into different files.
Just use a boolean instead.
Previously two different parts of the context had to be passed
separately; there were two sources of truth.
TyCtxt is a reference type and so can be passed by value.
`evaluate_obligation` can only be run on types that are already valid.
So rustdoc still has to run typeck even though it doesn't care about the
result.
…r=Mark-Simulacrum

Set shell for github actions CI

r? @pietroalbini but because this seems at worst harmless and CI is broken self approving
@Manishearth
Copy link
Member Author

@rustbot modify labels: +rollup
@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented Jul 16, 2020

📌 Commit ff685f5 has been approved by Manishearth

@rustbot rustbot added the rollup A PR which is a rollup label Jul 16, 2020
@bors
Copy link
Contributor

bors commented Jul 16, 2020

🌲 The tree is currently closed for pull requests below priority 5, this pull request will be tested once the tree is reopened

@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 Jul 16, 2020
@bors
Copy link
Contributor

bors commented Jul 16, 2020

⌛ Testing commit ff685f5 with merge 25f12cda027f2549831509e2602d0a81dc8b482e...

@bors
Copy link
Contributor

bors commented Jul 16, 2020

💔 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 Jul 16, 2020
@jyn514
Copy link
Member

jyn514 commented Jul 16, 2020

Looks like the cargo test suite?

 failures:

---- build::close_output stdout ----
thread 'build::close_output' panicked at 'assertion failed: !status.success()', src/tools/cargo/tests/testsuite/build.rs:5016:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    build::close_output

@Manishearth
Copy link
Member Author

I recall seeing that failure before!

@Manishearth
Copy link
Member Author

#74312 (comment)

@Manishearth
Copy link
Member Author

@bors retry

might be an intermittent, no common PRs

@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 Jul 16, 2020
@bors
Copy link
Contributor

bors commented Jul 16, 2020

⌛ Testing commit ff685f5 with merge 5c9e5df...

@jyn514
Copy link
Member

jyn514 commented Jul 16, 2020

Should we let the cargo team know?

@Manishearth
Copy link
Member Author

Yes, once we've determined it's actually an intermittent

@bors
Copy link
Contributor

bors commented Jul 16, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: Manishearth
Pushing 5c9e5df to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 16, 2020
@bors bors merged commit 5c9e5df into rust-lang:master Jul 16, 2020
@Manishearth Manishearth deleted the rollup-9gxn4od branch July 16, 2020 22:40
@cuviper cuviper added this to the 1.47.0 milestone May 2, 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. 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.