-
-
Notifications
You must be signed in to change notification settings - Fork 74
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
fix: never return type in let initializer #264
Conversation
Codecov Report
@@ Coverage Diff @@
## master #264 +/- ##
==========================================
+ Coverage 78.75% 79.13% +0.37%
==========================================
Files 215 214 -1
Lines 12857 12794 -63
==========================================
- Hits 10125 10124 -1
+ Misses 2732 2670 -62
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.
The fix looks good. I just have one question w.r.t. making the user aware of dead code. In the future we could potentially be dealing with side-effects, so we can do a similar recommendation to Rust. (i.e. use an _before_var_name
if you want the dead code to not raise a warning)
self.gen_let_statement(*pat, *initializer); | ||
// If the let statement never finishes, there is no need to generate more code | ||
if !self.gen_let_statement(*pat, *initializer) { | ||
return None; |
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.
Should we add a dead_code
diagnostic?
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.
Yes we should add that at some point but thats beyond the scope of this PR
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.
Is it though? I'm talking about at least adding the diagnostic
Adding hard opt-outs without giving any user feedback reduces usability. Moving this to a potential future improvement generally results in a large degradation before it's tackled (or even gets forgotten). It seems like this is the best time to also do that additional small change, instead of having inconveniences accumulate over time.
Solves an issue where if an initializer of a let statement has the never type it crashed the code generation.
Fixes #262