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 7 pull requests #134480

Closed
wants to merge 29 commits into from

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

jieyouxu and others added 29 commits December 17, 2024 19:09
- Move `tests/ui/attr-bad-crate-attr.rs` to `tests/ui/attributes/`.
- Briefly document test intent add link to relevant Reference docs.
- Move `tests/ui/attr-shebang.rs` to `tests/ui/attributes/`.
- Downgrade test to `check-pass`, this would fail very early if the
  parser did not accept `#![..]` attributes.
- Briefly document test intent.
- Move `tests/ui/attr-usage-inline.rs` to `tests/ui/attributes/inline/`.
- Briefly document test intent.
- Drop unnecessary `#[allow(dead_code)]` as this is allowed-by-default
  for ui test suite.
- Move `tests/ui/attrs-resolution-errors.rs` to `tests/ui/resolve/`.
- Document test intent.
- Rename test to `attr-macros-positional-rejection.rs` to better reflect
  test intent.
- Move `tests/ui/attrs-resolution.rs` to `tests/ui/resolve/`.
- Document test intent.
- Rename test to `non-macro-attrs-accepted.rs` to better reflect test
  intent.
This PR aims to improve the testing coverage for
`#[diagnostic::do_not_recommend]`. It ensures that all tests are run for
the old and new solver to verify that the behaviour is the same for both
variants. It also adds two new tests:

* A test with 4 traits having wild card impl for each other, with
alternating `#[diagnostic::do_not_recommend]` attributse
* A test with a lifetime dependend wild card impl, which is something
that's not supported yet
This commit adds a check that verifies that no arguments are passed to
`#[diagnostic::do_not_recommend]`. If we detect arguments we emit a warning.
This commit seeks to stabilize the `#[diagnostic::do_not_recommend]`
attribute.
This attribute was first proposed as `#[do_not_recommend`] attribute in
RFC 2397 (rust-lang/rfcs#2397). It gives the
crate authors the ability to not suggest to the compiler to not show
certain traits in it's error messages. With the presence of the
`#[diagnostic]` tool attribute namespace it was decided to move the
attribute there, as that lowers the amount of guarantees the compiler
needs to give about the exact way this influences error messages. It
turns the attribute into a hint which can be ignored. In addition to the
original proposed functionality this attribute now also hides the marked
trait in help messages ("This trait is implemented by: ").
The attribute does not accept any argument and can only be placed on
trait implementations. If it is placed somewhere else a lint warning is
emitted and the attribute is otherwise ignored. If an argument is
detected a lint warning is emitted and the argument is ignored. This
follows the rules outlined by the diagnostic namespace.

This attribute allows crates like diesel to improve their error messages
drastically. The most common example here is the following error
message:

```
error[E0277]: the trait bound `&str: Expression` is not satisfied
  --> /home/weiznich/Documents/rust/rust/tests/ui/diagnostic_namespace/do_not_recommend.rs:53:15
   |
LL |     SelectInt.check("bar");
   |               ^^^^^ the trait `Expression` is not implemented for `&str`, which is required by `&str: AsExpression<Integer>`
   |
   = help: the following other types implement trait `Expression`:
             Bound<T>
             SelectInt
note: required for `&str` to implement `AsExpression<Integer>`
  --> /home/weiznich/Documents/rust/rust/tests/ui/diagnostic_namespace/do_not_recommend.rs:26:13
   |
LL | impl<T, ST> AsExpression<ST> for T
   |             ^^^^^^^^^^^^^^^^     ^
LL | where
LL |     T: Expression<SqlType = ST>,
   |        ------------------------ unsatisfied trait bound introduced here
```

By applying the new attribute to the wild card trait implementation of
`AsExpression` for `T: Expression` the error message becomes:

```
error[E0277]: the trait bound `&str: AsExpression<Integer>` is not satisfied
  --> $DIR/as_expression.rs:55:15
   |
LL |     SelectInt.check("bar");
   |               ^^^^^ the trait `AsExpression<Integer>` is not implemented for `&str`
   |
   = help: the trait `AsExpression<Text>` is implemented for `&str`
   = help: for that trait implementation, expected `Text`, found `Integer`
```

which makes it much easier for users to understand that they are facing
a type mismatch.

Other explored example usages included

* This standard library error message: rust-lang#128008
* That bevy derived example:
https://github.com/rust-lang/rust/blob/e1f306899514ea80abc1d1c9f6a57762afb304a3/tests/ui/diagnostic_namespace/do_not_recommend/supress_suggestions_in_help.rs (No
more tuple pyramids)

Fixes rust-lang#51992
Signed-off-by: acceptacross <csqcqs@gmail.com>
…nd_final_tests, r=compiler-errors

Stabilize `#[diagnostic::do_not_recommend]`

This PR seeks to stabilize the `#[diagnostic::do_not_recommend]`attribute.

This attribute was first proposed as `#[do_not_recommend`] attribute in RFC 2397 (rust-lang/rfcs#2397). It gives the crate authors the ability to not suggest to the compiler to not show certain traits in its error messages.

With the presence of the `#[diagnostic]` tool attribute namespace it was decided to move the attribute there, as that lowers the amount of guarantees the compiler needs to give about the exact way this influences error messages. It turns the attribute into a hint which can be ignored. In addition to the original proposed functionality this attribute now also hides the marked trait in help messages ("This trait is implemented by: ").

The attribute does not accept any argument and can only be placed on trait implementations. If it is placed somewhere else a lint warning is emitted and the attribute is otherwise ignored. If an argument is detected a lint warning is emitted and the argument is ignored. This follows the rules outlined by the diagnostic namespace.

This attribute allows crates like diesel to improve their error messages drastically. The most common example here is the following error message:

```
error[E0277]: the trait bound `&str: Expression` is not satisfied
  --> /home/weiznich/Documents/rust/rust/tests/ui/diagnostic_namespace/do_not_recommend.rs:53:15
   |
LL |     SelectInt.check("bar");
   |               ^^^^^ the trait `Expression` is not implemented for `&str`, which is required by `&str: AsExpression<Integer>`
   |
   = help: the following other types implement trait `Expression`:
             Bound<T>
             SelectInt
note: required for `&str` to implement `AsExpression<Integer>`
  --> /home/weiznich/Documents/rust/rust/tests/ui/diagnostic_namespace/do_not_recommend.rs:26:13
   |
LL | impl<T, ST> AsExpression<ST> for T
   |             ^^^^^^^^^^^^^^^^     ^
LL | where
LL |     T: Expression<SqlType = ST>,
   |        ------------------------ unsatisfied trait bound introduced here
```

By applying the new attribute to the wild card trait implementation of
`AsExpression` for `T: Expression` the error message becomes:

```
error[E0277]: the trait bound `&str: AsExpression<Integer>` is not satisfied
  --> $DIR/as_expression.rs:55:15
   |
LL |     SelectInt.check("bar");
   |               ^^^^^ the trait `AsExpression<Integer>` is not implemented for `&str`
   |
   = help: the trait `AsExpression<Text>` is implemented for `&str`
   = help: for that trait implementation, expected `Text`, found `Integer`
```

which makes it much easier for users to understand that they are facing a type mismatch.

Other explored example usages include:

* This standard library error message: rust-lang#128008
* That bevy derived example:
https://github.com/rust-lang/rust/blob/e1f306899514ea80abc1d1c9f6a57762afb304a3/tests/ui/diagnostic_namespace/do_not_recommend/supress_suggestions_in_help.rs (No
more tuple pyramids)

Fixes rust-lang#51992

r? `@compiler-errors`

This PR also adds a few more tests, makes sure that all the tests are run for the old and new trait solver and adds a check that the attribute does not contain arguments.
…errors

-Znext-solver: modify candidate preference rules

This implements the design proposed in the FCP in rust-lang#132325 and matches the old solver behavior. I hope the inline comments are all sufficiently clear, I personally think this is a fairly clear improvement over the existing approach using `fn discard_impls_shadowed_by_env`. This fixes rust-lang/trait-system-refactor-initiative#96.

This also fixes rust-lang#133639 which encounters an ICE in negative coherence when evaluating the where-clause. Given the features required to trigger this ICE 🤷

r? `@compiler-errors`
…errors

Advent of `tests/ui` (misc cleanups and improvements) [3/N]

Part of rust-lang#133895.

Misc improvements to some ui tests immediately under `tests/ui/`.

Best reviewed commit-by-commit. Each commit's commit message contains further elaboration and rationale for changes.

r? compiler
…tes, r=notriddle

Fix intra doc links not generated inside footnote definitions

Fixes rust-lang#132208.

The problem was that we were running the `Footnote` "pass" before the `LinkReplacer` one. Sadly, the change is bigger than it should because we can't specialize the `Iterator` trait implementation, forcing me to add a new type to handle the other `Iterator` kind (the one which still has the `Range`).

r? `@notriddle`
…ler-errors

Forbid overwriting types in typeck

While trying to figure out some type setting logic in rust-lang#134248 I realized that we sometimes set a type twice. While hopefully that would have been the same type, we didn't ensure that at all and just silently accepted it. So now we reject setting it twice, unless errors are happening, then we don't care.

Best reviewed commit by commit.

No behaviour change is intended.
move lint_unused_mut into sub-fn

also, stop `mem::take`-ing stuff we only use by reference 🤷
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 18, 2024
@rustbot rustbot added 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) rollup A PR which is a rollup labels Dec 18, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=7

@bors
Copy link
Contributor

bors commented Dec 18, 2024

📌 Commit e01e132 has been approved by matthiaskrgr

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 Dec 18, 2024
@bors
Copy link
Contributor

bors commented Dec 18, 2024

⌛ Testing commit e01e132 with merge 367612f...

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 18, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#132056 (Stabilize `#[diagnostic::do_not_recommend]`)
 - rust-lang#133643 (-Znext-solver: modify candidate preference rules)
 - rust-lang#134418 (Advent of `tests/ui` (misc cleanups and improvements) [3/N])
 - rust-lang#134432 (Fix intra doc links not generated inside footnote definitions)
 - rust-lang#134473 (chore: fix some typos)
 - rust-lang#134474 (Forbid overwriting types in typeck)
 - rust-lang#134477 (move lint_unused_mut into sub-fn)

r? `@ghost`
`@rustbot` modify labels: rollup
@rust-log-analyzer
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot)
test [mir-opt] tests/mir-opt/unusual_item_types.rs ... ok

failures:

---- [mir-opt] tests/mir-opt/strip_debuginfo.rs#TINY stdout ----
thread '[mir-opt] tests/mir-opt/strip_debuginfo.rs#TINY' panicked at src/tools/compiletest/src/runtest.rs:213:30:
called `Result::unwrap()` on an `Err` value: Os { code: 17, kind: AlreadyExists, message: "File exists" }


failures:
    [mir-opt] tests/mir-opt/strip_debuginfo.rs#TINY

@bors
Copy link
Contributor

bors commented Dec 18, 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 Dec 18, 2024
@matthiaskrgr
Copy link
Member Author

#134351

@matthiaskrgr matthiaskrgr added the A-spurious Area: Spurious failures in builds (spuriously == for no apparent reason) label Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-spurious Area: Spurious failures in builds (spuriously == for no apparent reason) rollup A PR which is a rollup 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants