Skip to content

Query system cycle errors should be extendable with notes #53453

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

Open
tristanburgess opened this issue Aug 17, 2018 · 1 comment
Open

Query system cycle errors should be extendable with notes #53453

tristanburgess opened this issue Aug 17, 2018 · 1 comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@tristanburgess
Copy link
Contributor

tristanburgess commented Aug 17, 2018

Relevant PR: #53316
Relevant Issue: #52985

The query system automatically detects and emits a cycle error if a cycle occurs when dependency nodes are added to the query DAG. This error is extensible with a custom main message defined as below to help human readability,

impl<'tcx> QueryDescription<'tcx> for queries::normalize_ty_after_erasing_regions<'tcx> {

but is otherwise closed for modification outside of the query::plumbing module:
pub(super) fn report_cycle(self, CycleError { usage, cycle: stack }: CycleError<'gcx>)

It would be nice to have a mechanism in addition that allows custom notes and suggestions to be added to these errors to help illustrate why a cycle occurred, not just where. It may be possible to expose the DiagnosticBuilder or provide wrappers for methods like span_suggestion() and span_note()

@tristanburgess
Copy link
Contributor Author

It looks like one possible way to do this specifically for normalize_ty_after_erasing_regions is to add a try_normalize_ty_after_erasing_regions function to this list

impl<'a, 'tcx, 'lcx> TyCtxt<'a, 'tcx, 'lcx> {
, which would return a DiagnosticBuilder on cycle which can be modified before emitting, but there should be a cleaner way to do this kind of thing than to have to create new functions for each query that wants to offer this kind of control.

@estebank estebank added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 19, 2019
@jonas-schievink jonas-schievink added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jan 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants