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

link to the rustc-dev-guide on cycle errors #113184

Closed
jyn514 opened this issue Jun 30, 2023 · 4 comments · Fixed by #113622
Closed

link to the rustc-dev-guide on cycle errors #113184

jyn514 opened this issue Jun 30, 2023 · 4 comments · Fixed by #113622
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jyn514
Copy link
Member

jyn514 commented Jun 30, 2023

right now, new contributors to the compiler can be easily confused by the cycle error - queries are not common in other production compilers (that i know of) and i think it would make it a lot easier to understand if we linked to the docs: https://rustc-dev-guide.rust-lang.org/overview.html#queries / https://rustc-dev-guide.rust-lang.org/query.html

the error reporting has changed a bit since i last looked at it, but i think

#[derive(Diagnostic)]
#[diag(query_system_cycle, code = "E0391")]
pub struct Cycle {
#[primary_span]
pub span: Span,
pub stack_bottom: String,
#[subdiagnostic]
pub cycle_stack: Vec<CycleStack>,
#[subdiagnostic]
pub stack_count: StackCount,
#[subdiagnostic]
pub alias: Option<Alias>,
#[subdiagnostic]
pub cycle_usage: Option<CycleUsage>,
}
is the right place.

cc @jieyouxu

@jyn514 jyn514 added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Jun 30, 2023
@zaneduffield
Copy link

Hi @jyn514, is it normal to link to developer docs in compiler error reporting? Maybe it would be suitable to have this information in the output of rustc --explain E0391? The current output only shows an example of a cyclic trait dependency.

@jyn514
Copy link
Member Author

jyn514 commented Jul 9, 2023

@zaneduffield the cycle error is unlike other errors in that it's much more likely to trigger it by accident while working on the compiler itself.

i don't mind adding info to --explain but i think we should do that in addition to an inline link, not instead of.

@RickleAndMortimer
Copy link
Contributor

Hey everyone, I have a patch that adds the links in the --explain error code and compiler error. Is it alright if I submit a PR or are we still deciding on a approach currently?

@jyn514
Copy link
Member Author

jyn514 commented Jul 12, 2023

@RickleAndMortimer a PR would be great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants