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

fix: explain E0120 better cover cases when its raised #127949

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

princess-entrapta
Copy link

@princess-entrapta princess-entrapta commented Jul 19, 2024

Fixes #98996

Wording change on the explain of E0120 as requested

@rustbot
Copy link
Collaborator

rustbot commented Jul 19, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @lcnr (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@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. labels Jul 19, 2024
@tgross35
Copy link
Contributor

Thanks for the PR! While you're touching this file, mind updating the last example?

Alternatively, wrapping trait objects requires something:
```
trait MyTrait {}
//or Box<MyTrait>, if you wanted an owned trait object
struct MyWrapper<'a> { foo: &'a MyTrait }

Few changes there:

  1. It should be &'a dyn MyTrait nowadays, and Box<dyn MyTrait> in the comment
  2. "Alternatively, wrapping trait objects requires something" is pretty imprecise. I think just "Alternatively, Drop can be implemented on trait objects:" works, no need to explain what trait objects are (they are a lot easier to identify since dyn)
  3. Space between // and or

@tgross35
Copy link
Contributor

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 19, 2024
@tgross35
Copy link
Contributor

Also you can add "Fixes #98996" to your PR description to automatically close that when this gets merged.

@tgross35
Copy link
Contributor

Thanks! Add the backticks and squash into one commit please, then lgtm

@princess-entrapta princess-entrapta force-pushed the master branch 2 times, most recently from 8fbc5cb to a6cb41a Compare July 19, 2024 09:01
@rust-log-analyzer

This comment has been minimized.

@tgross35
Copy link
Contributor

And tidy failed, so these lines need a rewrap lol https://github.com/rust-lang/rust/actions/runs/10005456061/job/27656226565?pr=127949#step:25:1676 (you can run ./x test tidy to check locally). Thanks for updating this about 15 times

@rustbot rustbot added A-meta Area: Issues & PRs about the rust-lang/rust repository itself A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Jul 19, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jul 19, 2024

Some changes occurred in match lowering

cc @Nadrieril

triagebot.toml has been modified, there may have been changes to the review queue.

cc @davidtwco, @wesleywiser

Some changes occurred in match checking

cc @Nadrieril

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

This PR modifies tests/run-make/. If this PR is trying to port a Makefile
run-make test to use rmake.rs, please update the
run-make port tracking issue
so we can track our progress. You can either modify the tracking issue
directly, or you can comment on the tracking issue and link this PR.

cc @jieyouxu

Some changes occurred in exhaustiveness checking

cc @Nadrieril

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

These commits modify compiler targets.
(See the Target Tier Policy.)

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

@tgross35
Copy link
Contributor

Woah, rebase gone wrong?

@tgross35
Copy link
Contributor

@princess-entrapta the file itself looks good, but it seems like the git history got messed up. Maybe double check that master is up to date with the rust-lang repo (not your fork) and then rebase again. If these extra commits show up on your local history, you might just be able to drop them during rebase.

@princess-entrapta
Copy link
Author

Alrigrt, i check that now

@rustbot rustbot added the has-merge-commits PR has merge commits, merge with caution. label Jul 19, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jul 19, 2024

There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

You can start a rebase with the following commands:

$ # rebase
$ git pull --rebase https://github.com/rust-lang/rust.git master
$ git push --force-with-lease

The following commits are merge commits:

@rustbot rustbot added O-unix Operating system: Unix-like T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-style Relevant to the style team, which will review and decide on the PR/issue. labels Jul 19, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jul 19, 2024

changes to the core type system

cc @compiler-errors, @lcnr

changes to the core type system

cc @compiler-errors, @lcnr

Some changes occurred in src/doc/style-guide

cc @rust-lang/style

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

The run-make-support library was changed

cc @jieyouxu

HIR ty lowering was modified

cc @fmease

@rustbot rustbot removed the has-merge-commits PR has merge commits, merge with caution. label Jul 19, 2024
@princess-entrapta
Copy link
Author

It took me some time, idk what i've done wrong with the rebases, but it should be good now

@tgross35 tgross35 added A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools and removed A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. A-meta Area: Issues & PRs about the rust-lang/rust repository itself O-unix Operating system: Unix-like T-style Relevant to the style team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) A-run-make Area: port run-make Makefiles to rmake.rs labels Jul 19, 2024
@tgross35
Copy link
Contributor

It looks all good now. Thanks for the change!

r? @tgross35
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 19, 2024

📌 Commit af7ecb6 has been approved by tgross35

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 19, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 19, 2024
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#127295 (CFI: Support provided methods on traits)
 - rust-lang#127814 (`C-cmse-nonsecure-call`: improved error messages)
 - rust-lang#127949 (fix: explain E0120 better cover cases when its raised)
 - rust-lang#127966 (Use structured suggestions for unconstrained generic parameters on impl blocks)
 - rust-lang#127976 (Lazy type aliases: Diagostics: Detect bivariant ty params that are only used recursively)
 - rust-lang#127978 (Avoid ref when using format! for perf)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 41d3cb6 into rust-lang:master Jul 19, 2024
6 of 9 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jul 19, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 19, 2024
Rollup merge of rust-lang#127949 - princess-entrapta:master, r=tgross35

fix: explain E0120 better cover cases when its raised

Fixes rust-lang#98996

Wording change on the explain of E0120 as requested
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--explain E0120 constrained much more than how it's emitted
7 participants