Refuse a terminal iterator op over a closure iterator, and report instead of crash
`find`/`any`/`all`/`position` build an index loop from the chain's length. A closure iterator has no length -- it is iterated directly -- and `len()` answers `"0"` for one, so the loop ran `0 ..< 0` and the operation returned `None`, `false` or `true` as though the sequence were empty. That is the kind of wrong that looks like an answer, and it was introduced with those operations a few commits ago. Found because `bind_pattern` panicked on the same input. That `unreachable!()` is now a message: it is an invariant about *input*, and a crash is a worse answer than a report however confident the invariant -- the panic also said nothing about which adaptor it was, which is why it took a detour to find. Also adds `Result::ok` and `Result::err`, which discard the other half. Nim needs the discarded type named, since an `Option` of nothing is not a type. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
08793f0 parent: 6e2a88e modified
src/lower.rs +64 -7 | @@ -5338,6 +5338,18 @@ impl Lowerer { | ||
| 5338 | 5338 | None => return Ok(None), |
| 5339 | 5339 | }; |
| 5340 | 5340 | |
| 5341 | + // A closure iterator has no length -- it is iterated directly -- and | |
| 5342 | + // the index loop below would run `0 ..< 0` and answer as though the | |
| 5343 | + // sequence were empty. That is the kind of wrong that looks like an | |
| 5344 | + // answer, so it is refused. | |
| 5345 | + if matches!(it, Iter::Closure { .. }) { | |
| 5346 | + return Err(format!( | |
| 5347 | + "`.{name}()` over a closure iterator is not implemented: it has \ | |
| 5348 | + no length to loop over, and treating it as empty would answer \ | |
| 5349 | + as though the sequence were" | |
| 5350 | + )); | |
| 5351 | + } | |
| 5352 | + | |
| 5341 | 5353 | let idx = self.fresh("Idx"); |
| 5342 | 5354 | let out = self.fresh(match name { |
| 5343 | 5355 | "find" => "Find", |
| @@ -5841,14 +5853,16 @@ impl Lowerer { | ||
| 5841 | 5853 | _ => Err("unsupported `for` pattern over `filter`".into()), |
| 5842 | 5854 | } |
| 5843 | 5855 | } |
| 5856 | + // `.take(n)` shortens the loop and changes nothing about what each | |
| 5857 | + // element is, so the binding is the inner chain's. Before the | |
| 5858 | + // wildcard and the reference cases, which would otherwise consume | |
| 5859 | + // the pattern and leave `Take` to the arms that do not expect it. | |
| 5860 | + (p, Iter::Take { inner, .. }) => self.bind_pattern(p, inner, i), | |
| 5844 | 5861 | (Pat::Wild(_), _) => Ok(()), |
| 5845 | 5862 | // `for &byte in xs` — the `&` destructures the reference, which in |
| 5846 | 5863 | // Nim is already the value. |
| 5847 | 5864 | (Pat::Reference(r), _) => self.bind_pattern(&r.pat, it, i), |
| 5848 | 5865 | (Pat::Paren(p), _) => self.bind_pattern(&p.pat, it, i), |
| 5849 | - // `.take(n)` shortens the loop and changes nothing about what each | |
| 5850 | - // element is, so the binding is the inner chain's. | |
| 5851 | - (p, Iter::Take { inner, .. }) => self.bind_pattern(p, inner, i), | |
| 5852 | 5866 | (Pat::Ident(id), _) => { |
| 5853 | 5867 | let name = id.ident.to_string(); |
| 5854 | 5868 | match it { |
| @@ -5918,10 +5932,25 @@ impl Lowerer { | ||
| 5918 | 5932 | ); |
| 5919 | 5933 | Ok(()) |
| 5920 | 5934 | } |
| 5921 | - // Handled above. | |
| 5922 | - Iter::Zip(..) | Iter::Enumerate(..) | Iter::Map { .. } | |
| 5923 | - | Iter::Filter { .. } | Iter::Closure { .. } | |
| 5924 | - | Iter::Take { .. } => unreachable!(), | |
| 5935 | + // Handled by an earlier arm. Reported rather than | |
| 5936 | + // asserted: this is an invariant about *input*, and a | |
| 5937 | + // crash is a worse answer than a message however | |
| 5938 | + // confident the invariant. | |
| 5939 | + other => { | |
| 5940 | + return Err(format!( | |
| 5941 | + "internal: `for` binding over {} reached the \ | |
| 5942 | + single-name case, which does not handle it", | |
| 5943 | + match other { | |
| 5944 | + Iter::Zip(..) => "zip", | |
| 5945 | + Iter::Enumerate(..) => "enumerate", | |
| 5946 | + Iter::Map { .. } => "map", | |
| 5947 | + Iter::Filter { .. } => "filter", | |
| 5948 | + Iter::Closure { .. } => "a closure iterator", | |
| 5949 | + Iter::Take { .. } => "take", | |
| 5950 | + _ => "an adaptor", | |
| 5951 | + } | |
| 5952 | + )) | |
| 5953 | + } | |
| 5925 | 5954 | } |
| 5926 | 5955 | } |
| 5927 | 5956 | _ => Err("unsupported `for` pattern".into()), |
| @@ -9147,6 +9176,34 @@ impl Lowerer { | ||
| 9147 | 9176 | { |
| 9148 | 9177 | (recv.code.clone(), rt.clone()) |
| 9149 | 9178 | } |
| 9179 | + // `Result::ok` / `Result::err` discard the other half. Nim needs | |
| 9180 | + // the discarded type named, since an `Option` of nothing is not a | |
| 9181 | + // type -- which is why both are written out rather than shared. | |
| 9182 | + "ok" | "err" | |
| 9183 | + if m.args.is_empty() | |
| 9184 | + && matches!(&rt, Some(Nim::Named(n, a)) if n == "Result" && a.len() == 2) => | |
| 9185 | + { | |
| 9186 | + let (t, e) = match &rt { | |
| 9187 | + Some(Nim::Named(_, a)) => (a[0].clone(), a[1].clone()), | |
| 9188 | + _ => unreachable!(), | |
| 9189 | + }; | |
| 9190 | + let tmp = self.fresh("Ok"); | |
| 9191 | + self.line(&format!("let {} = {}", tmp, recv.code)); | |
| 9192 | + let (keep, code) = if name == "ok" { | |
| 9193 | + (t.clone(), format!( | |
| 9194 | + "(if {0}.ok: rsSome[{1}]({0}.val) else: rsNone[{1}]())", | |
| 9195 | + tmp, | |
| 9196 | + t.render() | |
| 9197 | + )) | |
| 9198 | + } else { | |
| 9199 | + (e.clone(), format!( | |
| 9200 | + "(if {0}.ok: rsNone[{1}]() else: rsSome[{1}]({0}.err))", | |
| 9201 | + tmp, | |
| 9202 | + e.render() | |
| 9203 | + )) | |
| 9204 | + }; | |
| 9205 | + (code, Some(Nim::Named("Option".into(), vec![keep]))) | |
| 9206 | + } | |
| 9150 | 9207 | // `unwrap_or_default` is the payload, or its zero value. |
| 9151 | 9208 | "unwrap_or_default" if m.args.is_empty() => { |
| 9152 | 9209 | let inner = match &rt { |
| @@ -5338,6 +5338,18 @@ impl Lowerer { | |||
| 5338 | None => return Ok(None), | 5338 | None => return Ok(None), |
| 5339 | }; | 5339 | }; |
| 5340 | 5340 | ||
| 5341 | + // A closure iterator has no length -- it is iterated directly -- and | ||
| 5342 | + // the index loop below would run `0 ..< 0` and answer as though the | ||
| 5343 | + // sequence were empty. That is the kind of wrong that looks like an | ||
| 5344 | + // answer, so it is refused. | ||
| 5345 | + if matches!(it, Iter::Closure { .. }) { | ||
| 5346 | + return Err(format!( | ||
| 5347 | + "`.{name}()` over a closure iterator is not implemented: it has \ | ||
| 5348 | + no length to loop over, and treating it as empty would answer \ | ||
| 5349 | + as though the sequence were" | ||
| 5350 | + )); | ||
| 5351 | + } | ||
| 5352 | + | ||
| 5341 | let idx = self.fresh("Idx"); | 5353 | let idx = self.fresh("Idx"); |
| 5342 | let out = self.fresh(match name { | 5354 | let out = self.fresh(match name { |
| 5343 | "find" => "Find", | 5355 | "find" => "Find", |
| @@ -5841,14 +5853,16 @@ impl Lowerer { | |||
| 5841 | _ => Err("unsupported `for` pattern over `filter`".into()), | 5853 | _ => Err("unsupported `for` pattern over `filter`".into()), |
| 5842 | } | 5854 | } |
| 5843 | } | 5855 | } |
| 5856 | + // `.take(n)` shortens the loop and changes nothing about what each | ||
| 5857 | + // element is, so the binding is the inner chain's. Before the | ||
| 5858 | + // wildcard and the reference cases, which would otherwise consume | ||
| 5859 | + // the pattern and leave `Take` to the arms that do not expect it. | ||
| 5860 | + (p, Iter::Take { inner, .. }) => self.bind_pattern(p, inner, i), | ||
| 5844 | (Pat::Wild(_), _) => Ok(()), | 5861 | (Pat::Wild(_), _) => Ok(()), |
| 5845 | // `for &byte in xs` — the `&` destructures the reference, which in | 5862 | // `for &byte in xs` — the `&` destructures the reference, which in |
| 5846 | // Nim is already the value. | 5863 | // Nim is already the value. |
| 5847 | (Pat::Reference(r), _) => self.bind_pattern(&r.pat, it, i), | 5864 | (Pat::Reference(r), _) => self.bind_pattern(&r.pat, it, i), |
| 5848 | (Pat::Paren(p), _) => self.bind_pattern(&p.pat, it, i), | 5865 | (Pat::Paren(p), _) => self.bind_pattern(&p.pat, it, i), |
| 5849 | - // `.take(n)` shortens the loop and changes nothing about what each | ||
| 5850 | - // element is, so the binding is the inner chain's. | ||
| 5851 | - (p, Iter::Take { inner, .. }) => self.bind_pattern(p, inner, i), | ||
| 5852 | (Pat::Ident(id), _) => { | 5866 | (Pat::Ident(id), _) => { |
| 5853 | let name = id.ident.to_string(); | 5867 | let name = id.ident.to_string(); |
| 5854 | match it { | 5868 | match it { |
| @@ -5918,10 +5932,25 @@ impl Lowerer { | |||
| 5918 | ); | 5932 | ); |
| 5919 | Ok(()) | 5933 | Ok(()) |
| 5920 | } | 5934 | } |
| 5921 | - // Handled above. | 5935 | + // Handled by an earlier arm. Reported rather than |
| 5922 | - Iter::Zip(..) | Iter::Enumerate(..) | Iter::Map { .. } | 5936 | + // asserted: this is an invariant about *input*, and a |
| 5923 | - | Iter::Filter { .. } | Iter::Closure { .. } | 5937 | + // crash is a worse answer than a message however |
| 5924 | - | Iter::Take { .. } => unreachable!(), | 5938 | + // confident the invariant. |
| 5939 | + other => { | ||
| 5940 | + return Err(format!( | ||
| 5941 | + "internal: `for` binding over {} reached the \ | ||
| 5942 | + single-name case, which does not handle it", | ||
| 5943 | + match other { | ||
| 5944 | + Iter::Zip(..) => "zip", | ||
| 5945 | + Iter::Enumerate(..) => "enumerate", | ||
| 5946 | + Iter::Map { .. } => "map", | ||
| 5947 | + Iter::Filter { .. } => "filter", | ||
| 5948 | + Iter::Closure { .. } => "a closure iterator", | ||
| 5949 | + Iter::Take { .. } => "take", | ||
| 5950 | + _ => "an adaptor", | ||
| 5951 | + } | ||
| 5952 | + )) | ||
| 5953 | + } | ||
| 5925 | } | 5954 | } |
| 5926 | } | 5955 | } |
| 5927 | _ => Err("unsupported `for` pattern".into()), | 5956 | _ => Err("unsupported `for` pattern".into()), |
| @@ -9147,6 +9176,34 @@ impl Lowerer { | |||
| 9147 | { | 9176 | { |
| 9148 | (recv.code.clone(), rt.clone()) | 9177 | (recv.code.clone(), rt.clone()) |
| 9149 | } | 9178 | } |
| 9179 | + // `Result::ok` / `Result::err` discard the other half. Nim needs | ||
| 9180 | + // the discarded type named, since an `Option` of nothing is not a | ||
| 9181 | + // type -- which is why both are written out rather than shared. | ||
| 9182 | + "ok" | "err" | ||
| 9183 | + if m.args.is_empty() | ||
| 9184 | + && matches!(&rt, Some(Nim::Named(n, a)) if n == "Result" && a.len() == 2) => | ||
| 9185 | + { | ||
| 9186 | + let (t, e) = match &rt { | ||
| 9187 | + Some(Nim::Named(_, a)) => (a[0].clone(), a[1].clone()), | ||
| 9188 | + _ => unreachable!(), | ||
| 9189 | + }; | ||
| 9190 | + let tmp = self.fresh("Ok"); | ||
| 9191 | + self.line(&format!("let {} = {}", tmp, recv.code)); | ||
| 9192 | + let (keep, code) = if name == "ok" { | ||
| 9193 | + (t.clone(), format!( | ||
| 9194 | + "(if {0}.ok: rsSome[{1}]({0}.val) else: rsNone[{1}]())", | ||
| 9195 | + tmp, | ||
| 9196 | + t.render() | ||
| 9197 | + )) | ||
| 9198 | + } else { | ||
| 9199 | + (e.clone(), format!( | ||
| 9200 | + "(if {0}.ok: rsNone[{1}]() else: rsSome[{1}]({0}.err))", | ||
| 9201 | + tmp, | ||
| 9202 | + e.render() | ||
| 9203 | + )) | ||
| 9204 | + }; | ||
| 9205 | + (code, Some(Nim::Named("Option".into(), vec![keep]))) | ||
| 9206 | + } | ||
| 9150 | // `unwrap_or_default` is the payload, or its zero value. | 9207 | // `unwrap_or_default` is the payload, or its zero value. |
| 9151 | "unwrap_or_default" if m.args.is_empty() => { | 9208 | "unwrap_or_default" if m.args.is_empty() => { |
| 9152 | let inner = match &rt { | 9209 | let inner = match &rt { |