Skip to content

add links to query documentation for E0391 #113622

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

Merged
merged 7 commits into from
Jul 20, 2023

Conversation

RickleAndMortimer
Copy link
Contributor

This PR adds links to https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for the rustc --explain E0391 and within the compiler error itself.

Fixes: #113184

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @compiler-errors (or someone else) soon.

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 A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) 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-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jul 12, 2023
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if I totally agree that we should be linking to the rustc-dev-guide for something that is a user-facing error.

@compiler-errors
Copy link
Member

r? diagnostics

cc @jyn514

@rustbot rustbot assigned oli-obk and unassigned compiler-errors Jul 12, 2023
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@RickleAndMortimer
Copy link
Contributor Author

RickleAndMortimer commented Jul 13, 2023

Hmm, should I implement the new error message on miri as well or is there a mistake in my code? The CI seems to fail when running tests on miri here

@jyn514
Copy link
Member

jyn514 commented Jul 13, 2023

you should bless the miri tests as well, since they depend on the internals you're changing.

@rustbot
Copy link
Collaborator

rustbot commented Jul 13, 2023

The Miri subtree was changed

cc @rust-lang/miri

@RickleAndMortimer
Copy link
Contributor Author

@rustbot review

Copy link
Contributor

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase over the latest master branch to eliminate the merge commit. You can also squash all commits into one if that's easier

@oli-obk
Copy link
Contributor

oli-obk commented Jul 18, 2023

The guide section for rebasing can be found here

We try to avoid merge commits in PRs as much as possible

@oli-obk oli-obk 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 18, 2023
@rustbot rustbot added the A-rustdoc-json Area: Rustdoc JSON backend label Jul 18, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Jul 18, 2023

#113622 (comment) is still unaddressed

@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 18, 2023
@RickleAndMortimer
Copy link
Contributor Author

image
Would placing the note like this suffice? I'm also trying to see if I can push the note back as it's own subdiagnostic but I'm not sure if I'm doing it correctly. Currently my fix adds

pub cycle_usage: Option<CycleUsage>,
#[note]
pub note_span: (),
}
to the Cycle Diagnostic but I was wondering how I can modify my commit to make the note it's own diagnostic

@RickleAndMortimer
Copy link
Contributor Author

@rustbot review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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, 2023
@rust-log-analyzer

This comment has been minimized.

@jyn514
Copy link
Member

jyn514 commented Jul 19, 2023

Currently my fix adds to the Cycle Diagnostic but I was wondering how I can modify my commit to make the note it's own diagnostic

fwiw i think it looks great as-is :) i don't think it needs a separate diagnostic.

@oli-obk
Copy link
Contributor

oli-obk commented Jul 19, 2023

This lgtm now, you just need to update rustdoc-ui tests: x test --bless rustdoc-ui

@oli-obk oli-obk 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, 2023
@RickleAndMortimer
Copy link
Contributor Author

@rustbot review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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, 2023
@jyn514
Copy link
Member

jyn514 commented Jul 19, 2023

@bors r=oli-obk

@bors
Copy link
Collaborator

bors commented Jul 19, 2023

📌 Commit a903ac5 has been approved by oli-obk

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 Jul 19, 2023
@bors
Copy link
Collaborator

bors commented Jul 20, 2023

⌛ Testing commit a903ac5 with merge 0646a5d...

@bors
Copy link
Collaborator

bors commented Jul 20, 2023

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 0646a5d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 20, 2023
@bors bors merged commit 0646a5d into rust-lang:master Jul 20, 2023
@rustbot rustbot added this to the 1.73.0 milestone Jul 20, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (0646a5d): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.3% [-2.3%, -2.3%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -2.3% [-2.3%, -2.3%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 649.527s -> 649.782s (0.04%)

@RickleAndMortimer RickleAndMortimer deleted the issue-113184-fix branch August 14, 2023 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) merged-by-bors This PR was explicitly merged by bors. 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. 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.

link to the rustc-dev-guide on cycle errors
8 participants