Skip to content

fix: never return type in let initializer#264

Merged
Wodann merged 1 commit intomun-lang:masterfrom
baszalmstra:fix/issue_262
Sep 6, 2020
Merged

fix: never return type in let initializer#264
Wodann merged 1 commit intomun-lang:masterfrom
baszalmstra:fix/issue_262

Conversation

@baszalmstra
Copy link
Collaborator

@baszalmstra baszalmstra commented Sep 5, 2020

Solves an issue where if an initializer of a let statement has the never type it crashed the code generation.

Fixes #262

@codecov
Copy link

codecov bot commented Sep 5, 2020

Codecov Report

Merging #264 into master will increase coverage by 0.37%.
The diff coverage is 81.81%.

Impacted file tree graph

@@            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     
Impacted Files Coverage Δ
crates/mun_codegen/src/ir/body.rs 84.23% <80.00%> (-0.14%) ⬇️
crates/mun_codegen/src/test.rs 96.58% <100.00%> (+0.02%) ⬆️
crates/tools/src/lib.rs 70.96% <0.00%> (-1.76%) ⬇️
crates/mun_language_server/src/main_loop.rs 23.21% <0.00%> (-0.35%) ⬇️
crates/mun/src/ops/init.rs 100.00% <0.00%> (ø)
crates/mun_codegen_macros/src/lib.rs
crates/mun_compiler/src/driver.rs 59.76% <0.00%> (+0.11%) ⬆️
crates/mun_target/src/abi/mod.rs 57.28% <0.00%> (+0.55%) ⬆️
crates/mun_libloader/src/lib.rs 73.91% <0.00%> (+1.91%) ⬆️
crates/mun_codegen/src/linker.rs 25.33% <0.00%> (+2.98%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d9ebc18...63239a6. Read the comment docs.

Copy link
Collaborator

@Wodann Wodann left a 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;
Copy link
Collaborator

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?

Copy link
Collaborator Author

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

Copy link
Collaborator

@Wodann Wodann Sep 6, 2020

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.

@Wodann Wodann merged commit 5b80df4 into mun-lang:master Sep 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: fix Bug fix or report

Projects

None yet

Development

Successfully merging this pull request may close these issues.

'main' panicked at 'internal error: entered unreachable code

2 participants