Uh oh!
There was an error while loading. Please reload this page.
Suggest Default::default() when binding isn't initialized - #102184
Conversation
rust-highfive
commented
Sep 23, 2022
r? @nagisa (rust-highfive has picked a reviewer for you, use r? to override) |
chenyukang
commented
Sep 23, 2022
My first try on the type-checking part, there are several parts I'm not sure. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
8356aa8 to
2bfab8eCompareUh oh!
There was an error while loading. Please reload this page.
| let mut visitor = LetVisitor { decl_span, ty_span: None }; | ||
| visitor.visit_body(&body); | ||
| if let Some(ty_span) = visitor.ty_span { |
There was a problem hiding this comment.
Hm, seems quite unfortunate that we only handle the bindings that have a specified type. As such code like this isn’t really covered:
fnapple(_:u32){}fnbanana(){let chaenomeles;apple(chaenomeles);}Not saying though that this needs to be implemented here and now, but perhaps a FIXME comment or an issue for this would be warranted.
There was a problem hiding this comment.
Fixed the code to support this scenario now, the type infer know it's a u32, so here we give this suggestion:
error[E0381]: used binding `chaenomeles` isn't initialized
--> bug.rs:5:11
|
4 | let chaenomeles;
| ----------- binding declared here but left uninitialized
5 | apple(chaenomeles);
| ^^^^^^^^^^^ `chaenomeles` used here but it isn't initialized
|
help: consider assigning a value
|
4 | let chaenomeles = 0;
| +++Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
85582e6 to
21b9222Comparenagisa
commented
Sep 24, 2022
This is quite awesome! @bors r+ |
bors
commented
Sep 24, 2022
…sugg, r=nagisa Suggest Default::default() when binding isn't initialized Fixesrust-lang#102087
matthiaskrgr
commented
Sep 25, 2022
@bors r- failed in a rollup (test needs update) |
updated it! |
nagisa
commented
Sep 25, 2022
Unfortunately that’s the downside of only running the tests on the specified architecture. The more appropriate thing to do is to write the test in a way that is able to cross-compile if necessary to test target-specific details, but this isn’t always feasible (e.g. when test depends on The most straightfoward thing to do today is to just let bors deal with it, to be honest. @bors r+ rollup=never |
bors
commented
Sep 25, 2022
📌 Commit 8b2a008fd2edfd01c8f9ede3d5131dbea5d0d725 has been approved by It is now in the queue for this repository. |
nagisa
commented
Sep 25, 2022
@bors r- Could you squash the commits a little please? Commits like “cleanup” or “update test” don’t really carry themselves. |
8b2a008 to
672e3f4Comparechenyukang
commented
Sep 25, 2022
squashed! |
nagisa
commented
Sep 26, 2022
@bors r+ |
bors
commented
Sep 26, 2022
bors
commented
Sep 26, 2022
bors
commented
Sep 26, 2022
☀️ Test successful - checks-actions |
rust-timer
commented
Sep 26, 2022
Finished benchmarking commit (84946fe): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Footnotes |
Fixes#102087