From 282b063c6f069ed114a7a76dfed4b7efc995c232 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Mon, 22 Mar 2021 16:25:52 +0000 Subject: [PATCH 1/4] Provide test case for strange behaviour Signed-off-by: Ian Jackson --- src/lib.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 39177fa..52c9898 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -239,6 +239,10 @@ macro_rules! __if_chain { #[cfg(test)] mod tests { + #[derive(Debug,Copy,Clone,Eq,PartialEq)] + enum Got { Then(usize), Else(usize) } + use self::Got::*; + #[test] fn simple() { let x: Option, (u32, u32)>> = Some(Err((41, 42))); @@ -311,4 +315,23 @@ mod tests { }; assert_eq!(x, 3); } + + #[test] + fn weirdness() { + fn wat(seq: &[usize]) -> Got { + let mut seq = seq.iter().copied(); + let dunno = 0; + if_chain! { + if let Some(dunno) = seq.next(); + if let Some(dunno) = seq.next(); + then { Then(dunno) } + else { Else(dunno) } + } + } + + assert_eq!(Else(0), wat(&[ ])); + assert_eq!(Else(1), wat(&[ 1 ])); + assert_eq!(Then(2), wat(&[ 1, 2 ])); + assert_eq!(Then(2), wat(&[ 1, 2, 3 ])); + } } From 75a0aee5a9e5a76fe98355e85a6aac5cbc977ef6 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Mon, 22 Mar 2021 15:57:52 +0000 Subject: [PATCH 2/4] variable rebinding: Use Some() etc. to move scope of else If you rebind variables in the various ifs, the else should have none of them in scope. As a bonus this means that the expansion contains only one copy of the else clause. There is a behavioural change in some code, but arguably a bugfix. Signed-off-by: Ian Jackson --- src/lib.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 52c9898..eee03fc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -164,7 +164,10 @@ macro_rules! if_chain { macro_rules! __if_chain { // Expand with both a successful case and a fallback (@init ($($tt:tt)*) then { $($then:tt)* } else { $($other:tt)* }) => { - __if_chain! { @expand { $($other)* } $($tt)* then { $($then)* } } + match __if_chain! { @expand { None } $($tt)* then { Some({ $($then)* })} } { + Some(__if_chain_y) => __if_chain_y, + None => { $($other)* }, + } }; // Expand with no fallback (@init ($($tt:tt)*) then { $($then:tt)* }) => { @@ -317,12 +320,13 @@ mod tests { } #[test] - fn weirdness() { + fn let_rebinding_values() { + #[allow(unused_variables)] fn wat(seq: &[usize]) -> Got { let mut seq = seq.iter().copied(); let dunno = 0; if_chain! { - if let Some(dunno) = seq.next(); + if let Some(dunno) = seq.next(); // unused binding if let Some(dunno) = seq.next(); then { Then(dunno) } else { Else(dunno) } @@ -330,7 +334,7 @@ mod tests { } assert_eq!(Else(0), wat(&[ ])); - assert_eq!(Else(1), wat(&[ 1 ])); + assert_eq!(Else(0), wat(&[ 1 ])); assert_eq!(Then(2), wat(&[ 1, 2 ])); assert_eq!(Then(2), wat(&[ 1, 2, 3 ])); } From 880ac4a987a1ba8caaac5b96acfb7ffc18e960b1 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Mon, 22 Mar 2021 15:57:47 +0000 Subject: [PATCH 3/4] variable rebinding: add a test case Signed-off-by: Ian Jackson --- src/lib.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index eee03fc..283c52b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -338,4 +338,17 @@ mod tests { assert_eq!(Then(2), wat(&[ 1, 2 ])); assert_eq!(Then(2), wat(&[ 1, 2, 3 ])); } + + #[test] + fn let_rebinding() { + let v = Some(Some(Some('a'))); + if_chain! { + if let Some(v) = v; + if let Some(v) = v; + if let Some(v) = v; + if true; + then { let _: char = v; } + else { let _: Option>> = v; } + }; + } } From a0c035c6b16a655aff33c74c32b7c835f1da4d59 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Mon, 22 Mar 2021 17:05:37 +0000 Subject: [PATCH 4/4] Fix test case to compile on older Rust Iterator::copied() is too now. Signed-off-by: Ian Jackson --- src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 283c52b..7e6a50a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -323,11 +323,11 @@ mod tests { fn let_rebinding_values() { #[allow(unused_variables)] fn wat(seq: &[usize]) -> Got { - let mut seq = seq.iter().copied(); + let mut seq = seq.iter(); let dunno = 0; if_chain! { - if let Some(dunno) = seq.next(); // unused binding - if let Some(dunno) = seq.next(); + if let Some(&dunno) = seq.next(); // unused binding + if let Some(&dunno) = seq.next(); then { Then(dunno) } else { Else(dunno) } }