-
Notifications
You must be signed in to change notification settings - Fork 197
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
Remove lowering cruft from ingot/module items #727
Conversation
Codecov Report
@@ Coverage Diff @@
## master #727 +/- ##
==========================================
+ Coverage 81.92% 82.19% +0.27%
==========================================
Files 105 105
Lines 11822 11780 -42
==========================================
- Hits 9685 9683 -2
+ Misses 2137 2097 -40
Continue to review full report at Codecov.
|
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.
Seems nice!
In addition to the changes that you've made, we could remove
fe/crates/analyzer/src/context.rs
Line 380 in 43b58a8
pub var_decl_types: IndexMap<NodeId, Type>, |
fe/crates/analyzer/src/namespace/scopes.rs
Line 196 in 43b58a8
pub fn add_declaration(&self, node: &Node<ast::TypeDesc>, typ: Type) { |
var_decl_types
are referred by fe-lowering
, but it's no longer used. Also it wrongly stores type annotation spans as its key, instead of variables spans.
So I think it'd be better to remove them in favor of
fe/crates/analyzer/src/context.rs
Line 376 in 43b58a8
pub var_types: IndexMap<NodeId, Type>, |
Of course, it's completely fine if you merge this PR as is.
I like mindless work and I'll do it after mering the PR 😄
Gone! I also removed some other dead code, as revealed by https://github.com/est31/warnalyzer |
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.
👍
Left one nit.
fc3fc3c
to
2a839b5
Compare
What was wrong?
Closes #720 (I think; @Y-Nak, are you aware of any other leftorovers?)
The only functional change is that
db.set_ingot_modules
has been replaced withdb.set_ingot_files
. The former was a compromise to work with lowered modules. Now the latter will be used by the (imaginary) language server to update the list of files in a project.How was it fixed?
🧠
To-Do
OPTIONAL: Update Spec if applicable
Add entry to the release notes (may forgo for trivial changes)
Clean up commit history