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

Diagnostic refactor #1048

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Diagnostic refactor #1048

wants to merge 4 commits into from

Conversation

sbillig
Copy link
Collaborator

@sbillig sbillig commented Jan 24, 2025

Goal:

  • Diagnostic enums should contain the (salsa) ids of the things involved in the error (eg the TyIds of the types for a type mismatch). These enum variants should generally be constructed directly, rather than via constructor functions that might generate strings, etc.
  • The complexity of constructing helpful messages/labels for the user should be consolidated in the to_complete(..) -> CompleteDiagnostic functions.
  • We should add convenience functions to the (salsa) id types where it's helpful.

I scrapped the LazyDiagnostic idea that I showed during the meeting, in favor of just moving the diagnostic stuff from the hir crate to the hir-analysis crate, so now DiagnosticVoucher::to_complete takes a SpannedHirAnalysisDb instead of a SpannedHirDb, etc. Its useful to have concrete span locations when generating the diagnostic text, so we can sort the labels by location and use text like "x defined first here", "x redefined here", etc and have it make sense.

I'll submit some follow-up PRs that actually improve diagnostics in the future, and to do a little more code shuffling. I'd like to merge this because it touches a lot of lines and could lead to conflict hassles for other stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant