-
Notifications
You must be signed in to change notification settings - Fork 597
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
Report source locator in when scoping error messages #3804
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good and this is definitely an improvement.
This hints at missing infrastructure for pretty printing multiple source locators. E.g., it would be useful if after doing one line-carat error pointing at one bad operation, you could then show the when that it escaped from. Just showing a bare source locator is better.
Something like attachNote
(https://mlir.llvm.org/docs/Diagnostics/#attaching-notes) is what is ideal.
Yeah definitely. I thought about trying to tackle that when I was making that changed but just went with the smaller touch. These errors could also be changed to be non-fatal (report error rather than throwing exception). |
afd72d5
to
d7d9287
Compare
It also would be super nice to have the source locator for where the current data was bound (eg. where the |
(cherry picked from commit d21a663) # Conflicts: # core/src/main/scala/chisel3/Data.scala
(cherry picked from commit d21a663) # Conflicts: # core/src/main/scala/chisel3/Data.scala
(cherry picked from commit d21a663)
Not really a bugfix but that seemed closest
Contributor Checklist
docs/src
?Type of Improvement
Desired Merge Strategy
Release Notes
Reviewer Checklist (only modified by reviewer)
3.6.x
,5.x
, or6.x
depending on impact, API modification or big change:7.0
)?Enable auto-merge (squash)
, clean up the commit message, and label withPlease Merge
.Create a merge commit
.