Add `--report`: every item's failure in one run, not the first
Four batches of fixes moved the per-crate score not at all -- each crate has a queue of these and rustnim reported one per run, so a crate only passes when its whole queue empties, and until then the score says nothing happened. The loop was the bottleneck, not the fixes. `--report` collects every item's failure and prints them by cause. It is diagnostic only and writes no output: a file missing the items that failed is not a translation of anything. It immediately paid for itself. `color_quant`, which had looked like an open-ended series of failures, turned out to have *one* failing item at a time -- `panic`, then `.get()`, then `Vec::new()` -- each visible and fixable in seconds rather than one eight-minute run apiece. Also lowers `panic`/`assert`/`unreachable` (the expansions of the macros) to `rsPanic`, which raises the same `RustPanic` as every other panic here so the exit status and stderr still match; and `v.get(i)`, which is the bounds-checked index -- `Some` in range, `None` out of it, where `v[i]` panics. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
4172f11 parent: 9ee9823 modified
src/lower.rs +89 -2 | @@ -374,6 +374,11 @@ pub struct Lowerer { | ||
| 374 | 374 | /// -- which runs after all the files, not inside one -- can say where a |
| 375 | 375 | /// failure came from. |
| 376 | 376 | enum_module: HashMap<String, String>, |
| 377 | + /// Collect every item's failure instead of stopping at the first. | |
| 378 | + /// Diagnostic only: a run in this mode writes no output, because a file | |
| 379 | + /// missing the items that failed is not a translation of anything. | |
| 380 | + pub report: bool, | |
| 381 | + errors: Vec<String>, | |
| 377 | 382 | /// Bounds discovered while rewriting `impl Trait` parameters. `rewrite_apit` |
| 378 | 383 | /// runs from `&self`, so they are parked here and folded into `bounds` by |
| 379 | 384 | /// whoever called it. |
| @@ -476,6 +481,8 @@ impl Lowerer { | ||
| 476 | 481 | fn_blankets: HashMap::new(), |
| 477 | 482 | untranslatable: HashMap::new(), |
| 478 | 483 | enum_module: HashMap::new(), |
| 484 | + report: false, | |
| 485 | + errors: Vec::new(), | |
| 479 | 486 | pending_apit_bounds: std::cell::RefCell::new(Vec::new()), |
| 480 | 487 | bounds: HashMap::new(), |
| 481 | 488 | const_names_at: std::cell::Cell::new(None), |
| @@ -638,7 +645,14 @@ impl Lowerer { | ||
| 638 | 645 | for (m, f) in files { |
| 639 | 646 | self.cur_mod = m.clone(); |
| 640 | 647 | for item in &f.items { |
| 641 | - self.item_types(item).map_err(|e| self.where_(&e))?; | |
| 648 | + if let Err(e) = self.item_types(item) { | |
| 649 | + let e = self.where_(&e); | |
| 650 | + if self.report { | |
| 651 | + self.errors.push(e); | |
| 652 | + } else { | |
| 653 | + return Err(e); | |
| 654 | + } | |
| 655 | + } | |
| 642 | 656 | } |
| 643 | 657 | } |
| 644 | 658 | |
| @@ -671,7 +685,14 @@ impl Lowerer { | ||
| 671 | 685 | for (m, f) in files { |
| 672 | 686 | self.cur_mod = m.clone(); |
| 673 | 687 | for item in &f.items { |
| 674 | - self.item(item).map_err(|e| self.where_(&e))?; | |
| 688 | + if let Err(e) = self.item(item) { | |
| 689 | + let e = self.where_(&e); | |
| 690 | + if self.report { | |
| 691 | + self.errors.push(e); | |
| 692 | + } else { | |
| 693 | + return Err(e); | |
| 694 | + } | |
| 695 | + } | |
| 675 | 696 | } |
| 676 | 697 | } |
| 677 | 698 | |
| @@ -694,6 +715,37 @@ impl Lowerer { | ||
| 694 | 715 | )); |
| 695 | 716 | } |
| 696 | 717 | |
| 718 | + if self.report && !self.errors.is_empty() { | |
| 719 | + let mut counts: HashMap<String, usize> = HashMap::new(); | |
| 720 | + for e in &self.errors { | |
| 721 | + // The message without its quoted particulars, so the same | |
| 722 | + // gap reported about twenty different names counts once. | |
| 723 | + let head: String = e | |
| 724 | + .split('`') | |
| 725 | + .step_by(2) | |
| 726 | + .collect::<Vec<_>>() | |
| 727 | + .join("`X`") | |
| 728 | + .chars() | |
| 729 | + .take(100) | |
| 730 | + .collect(); | |
| 731 | + *counts.entry(head).or_default() += 1; | |
| 732 | + } | |
| 733 | + let mut hist: Vec<(String, usize)> = counts.into_iter().collect(); | |
| 734 | + hist.sort_by(|a, b| b.1.cmp(&a.1).then(a.0.cmp(&b.0))); | |
| 735 | + let mut out = format!("{} item(s) could not be translated.\n\nby cause:\n", self.errors.len()); | |
| 736 | + for (w, n) in hist.iter().take(40) { | |
| 737 | + out.push_str(&format!(" {n:>5} {w}\n")); | |
| 738 | + } | |
| 739 | + let mut msgs: Vec<String> = self.errors.clone(); | |
| 740 | + msgs.sort(); | |
| 741 | + msgs.dedup(); | |
| 742 | + out.push_str(&format!("\nfirst {} of {}:\n", msgs.len().min(30), msgs.len())); | |
| 743 | + for m in msgs.iter().take(30) { | |
| 744 | + out.push_str(&format!(" {m}\n")); | |
| 745 | + } | |
| 746 | + return Err(out); | |
| 747 | + } | |
| 748 | + | |
| 697 | 749 | if self.fns.contains_key(&(String::new(), "main".to_string())) { |
| 698 | 750 | self.blank(); |
| 699 | 751 | self.line("when isMainModule:"); |
| @@ -6811,6 +6863,19 @@ impl Lowerer { | ||
| 6811 | 6863 | } |
| 6812 | 6864 | } |
| 6813 | 6865 | } |
| 6866 | + // `assert!` and `unreachable!` expand to these. The message is | |
| 6867 | + // already built by the expansion; `rsPanic` raises the same | |
| 6868 | + // `RustPanic` every other panic here does, so the exit status and | |
| 6869 | + // stderr match. | |
| 6870 | + "panic" | "panic_fmt" | "panic_explicit" | "unreachable" | "assert" | |
| 6871 | + if codes.len() <= 1 => | |
| 6872 | + { | |
| 6873 | + let msg = codes | |
| 6874 | + .first() | |
| 6875 | + .cloned() | |
| 6876 | + .unwrap_or_else(|| "\"explicit panic\"".into()); | |
| 6877 | + return Ok(Val::new(format!("rsPanic({msg})"), Some(Nim::Unit))); | |
| 6878 | + } | |
| 6814 | 6879 | // `Ord::cmp` / `PartialOrd::partial_cmp`. Nim's `cmp` returns an |
| 6815 | 6880 | // `int` whose sign carries the same information, and `rsOrd` names |
| 6816 | 6881 | // the three cases. `partial_cmp` is the total one wrapped in |
| @@ -7309,6 +7374,28 @@ impl Lowerer { | ||
| 7309 | 7374 | Some(Nim::Prim(to.into())), |
| 7310 | 7375 | ) |
| 7311 | 7376 | } |
| 7377 | + // `v.get(i)` is the bounds-checked index: `Some` when it is in | |
| 7378 | + // range, `None` when it is not, where `v[i]` would panic. | |
| 7379 | + "get" if matches!(rt, Some(Nim::Seq(_)) | Some(Nim::OpenArray(_))) | |
| 7380 | + && m.args.len() == 1 => | |
| 7381 | + { | |
| 7382 | + let elem = match &rt { | |
| 7383 | + Some(Nim::Seq(e)) | Some(Nim::OpenArray(e)) => (**e).clone(), | |
| 7384 | + _ => unreachable!(), | |
| 7385 | + }; | |
| 7386 | + let i = a0.clone().unwrap_or_default(); | |
| 7387 | + let tmp = self.fresh("Get"); | |
| 7388 | + self.line(&format!("let {tmp} = int({i})")); | |
| 7389 | + ( | |
| 7390 | + format!( | |
| 7391 | + "(if {0} >= 0 and {0} < {1}.len: rsSome[{2}]({1}[{0}]) else: rsNone[{2}]())", | |
| 7392 | + tmp, | |
| 7393 | + recv.code, | |
| 7394 | + elem.render() | |
| 7395 | + ), | |
| 7396 | + Some(Nim::Named("Option".into(), vec![elem])), | |
| 7397 | + ) | |
| 7398 | + } | |
| 7312 | 7399 | "clear" if matches!(rt, Some(Nim::Seq(_)) | Some(Nim::Prim(_))) => { |
| 7313 | 7400 | (format!("{}.setLen(0)", recv.code), Some(Nim::Unit)) |
| 7314 | 7401 | } |
| @@ -374,6 +374,11 @@ pub struct Lowerer { | |||
| 374 | /// -- which runs after all the files, not inside one -- can say where a | 374 | /// -- which runs after all the files, not inside one -- can say where a |
| 375 | /// failure came from. | 375 | /// failure came from. |
| 376 | enum_module: HashMap<String, String>, | 376 | enum_module: HashMap<String, String>, |
| 377 | + /// Collect every item's failure instead of stopping at the first. | ||
| 378 | + /// Diagnostic only: a run in this mode writes no output, because a file | ||
| 379 | + /// missing the items that failed is not a translation of anything. | ||
| 380 | + pub report: bool, | ||
| 381 | + errors: Vec<String>, | ||
| 377 | /// Bounds discovered while rewriting `impl Trait` parameters. `rewrite_apit` | 382 | /// Bounds discovered while rewriting `impl Trait` parameters. `rewrite_apit` |
| 378 | /// runs from `&self`, so they are parked here and folded into `bounds` by | 383 | /// runs from `&self`, so they are parked here and folded into `bounds` by |
| 379 | /// whoever called it. | 384 | /// whoever called it. |
| @@ -476,6 +481,8 @@ impl Lowerer { | |||
| 476 | fn_blankets: HashMap::new(), | 481 | fn_blankets: HashMap::new(), |
| 477 | untranslatable: HashMap::new(), | 482 | untranslatable: HashMap::new(), |
| 478 | enum_module: HashMap::new(), | 483 | enum_module: HashMap::new(), |
| 484 | + report: false, | ||
| 485 | + errors: Vec::new(), | ||
| 479 | pending_apit_bounds: std::cell::RefCell::new(Vec::new()), | 486 | pending_apit_bounds: std::cell::RefCell::new(Vec::new()), |
| 480 | bounds: HashMap::new(), | 487 | bounds: HashMap::new(), |
| 481 | const_names_at: std::cell::Cell::new(None), | 488 | const_names_at: std::cell::Cell::new(None), |
| @@ -638,7 +645,14 @@ impl Lowerer { | |||
| 638 | for (m, f) in files { | 645 | for (m, f) in files { |
| 639 | self.cur_mod = m.clone(); | 646 | self.cur_mod = m.clone(); |
| 640 | for item in &f.items { | 647 | for item in &f.items { |
| 641 | - self.item_types(item).map_err(|e| self.where_(&e))?; | 648 | + if let Err(e) = self.item_types(item) { |
| 649 | + let e = self.where_(&e); | ||
| 650 | + if self.report { | ||
| 651 | + self.errors.push(e); | ||
| 652 | + } else { | ||
| 653 | + return Err(e); | ||
| 654 | + } | ||
| 655 | + } | ||
| 642 | } | 656 | } |
| 643 | } | 657 | } |
| 644 | 658 | ||
| @@ -671,7 +685,14 @@ impl Lowerer { | |||
| 671 | for (m, f) in files { | 685 | for (m, f) in files { |
| 672 | self.cur_mod = m.clone(); | 686 | self.cur_mod = m.clone(); |
| 673 | for item in &f.items { | 687 | for item in &f.items { |
| 674 | - self.item(item).map_err(|e| self.where_(&e))?; | 688 | + if let Err(e) = self.item(item) { |
| 689 | + let e = self.where_(&e); | ||
| 690 | + if self.report { | ||
| 691 | + self.errors.push(e); | ||
| 692 | + } else { | ||
| 693 | + return Err(e); | ||
| 694 | + } | ||
| 695 | + } | ||
| 675 | } | 696 | } |
| 676 | } | 697 | } |
| 677 | 698 | ||
| @@ -694,6 +715,37 @@ impl Lowerer { | |||
| 694 | )); | 715 | )); |
| 695 | } | 716 | } |
| 696 | 717 | ||
| 718 | + if self.report && !self.errors.is_empty() { | ||
| 719 | + let mut counts: HashMap<String, usize> = HashMap::new(); | ||
| 720 | + for e in &self.errors { | ||
| 721 | + // The message without its quoted particulars, so the same | ||
| 722 | + // gap reported about twenty different names counts once. | ||
| 723 | + let head: String = e | ||
| 724 | + .split('`') | ||
| 725 | + .step_by(2) | ||
| 726 | + .collect::<Vec<_>>() | ||
| 727 | + .join("`X`") | ||
| 728 | + .chars() | ||
| 729 | + .take(100) | ||
| 730 | + .collect(); | ||
| 731 | + *counts.entry(head).or_default() += 1; | ||
| 732 | + } | ||
| 733 | + let mut hist: Vec<(String, usize)> = counts.into_iter().collect(); | ||
| 734 | + hist.sort_by(|a, b| b.1.cmp(&a.1).then(a.0.cmp(&b.0))); | ||
| 735 | + let mut out = format!("{} item(s) could not be translated.\n\nby cause:\n", self.errors.len()); | ||
| 736 | + for (w, n) in hist.iter().take(40) { | ||
| 737 | + out.push_str(&format!(" {n:>5} {w}\n")); | ||
| 738 | + } | ||
| 739 | + let mut msgs: Vec<String> = self.errors.clone(); | ||
| 740 | + msgs.sort(); | ||
| 741 | + msgs.dedup(); | ||
| 742 | + out.push_str(&format!("\nfirst {} of {}:\n", msgs.len().min(30), msgs.len())); | ||
| 743 | + for m in msgs.iter().take(30) { | ||
| 744 | + out.push_str(&format!(" {m}\n")); | ||
| 745 | + } | ||
| 746 | + return Err(out); | ||
| 747 | + } | ||
| 748 | + | ||
| 697 | if self.fns.contains_key(&(String::new(), "main".to_string())) { | 749 | if self.fns.contains_key(&(String::new(), "main".to_string())) { |
| 698 | self.blank(); | 750 | self.blank(); |
| 699 | self.line("when isMainModule:"); | 751 | self.line("when isMainModule:"); |
| @@ -6811,6 +6863,19 @@ impl Lowerer { | |||
| 6811 | } | 6863 | } |
| 6812 | } | 6864 | } |
| 6813 | } | 6865 | } |
| 6866 | + // `assert!` and `unreachable!` expand to these. The message is | ||
| 6867 | + // already built by the expansion; `rsPanic` raises the same | ||
| 6868 | + // `RustPanic` every other panic here does, so the exit status and | ||
| 6869 | + // stderr match. | ||
| 6870 | + "panic" | "panic_fmt" | "panic_explicit" | "unreachable" | "assert" | ||
| 6871 | + if codes.len() <= 1 => | ||
| 6872 | + { | ||
| 6873 | + let msg = codes | ||
| 6874 | + .first() | ||
| 6875 | + .cloned() | ||
| 6876 | + .unwrap_or_else(|| "\"explicit panic\"".into()); | ||
| 6877 | + return Ok(Val::new(format!("rsPanic({msg})"), Some(Nim::Unit))); | ||
| 6878 | + } | ||
| 6814 | // `Ord::cmp` / `PartialOrd::partial_cmp`. Nim's `cmp` returns an | 6879 | // `Ord::cmp` / `PartialOrd::partial_cmp`. Nim's `cmp` returns an |
| 6815 | // `int` whose sign carries the same information, and `rsOrd` names | 6880 | // `int` whose sign carries the same information, and `rsOrd` names |
| 6816 | // the three cases. `partial_cmp` is the total one wrapped in | 6881 | // the three cases. `partial_cmp` is the total one wrapped in |
| @@ -7309,6 +7374,28 @@ impl Lowerer { | |||
| 7309 | Some(Nim::Prim(to.into())), | 7374 | Some(Nim::Prim(to.into())), |
| 7310 | ) | 7375 | ) |
| 7311 | } | 7376 | } |
| 7377 | + // `v.get(i)` is the bounds-checked index: `Some` when it is in | ||
| 7378 | + // range, `None` when it is not, where `v[i]` would panic. | ||
| 7379 | + "get" if matches!(rt, Some(Nim::Seq(_)) | Some(Nim::OpenArray(_))) | ||
| 7380 | + && m.args.len() == 1 => | ||
| 7381 | + { | ||
| 7382 | + let elem = match &rt { | ||
| 7383 | + Some(Nim::Seq(e)) | Some(Nim::OpenArray(e)) => (**e).clone(), | ||
| 7384 | + _ => unreachable!(), | ||
| 7385 | + }; | ||
| 7386 | + let i = a0.clone().unwrap_or_default(); | ||
| 7387 | + let tmp = self.fresh("Get"); | ||
| 7388 | + self.line(&format!("let {tmp} = int({i})")); | ||
| 7389 | + ( | ||
| 7390 | + format!( | ||
| 7391 | + "(if {0} >= 0 and {0} < {1}.len: rsSome[{2}]({1}[{0}]) else: rsNone[{2}]())", | ||
| 7392 | + tmp, | ||
| 7393 | + recv.code, | ||
| 7394 | + elem.render() | ||
| 7395 | + ), | ||
| 7396 | + Some(Nim::Named("Option".into(), vec![elem])), | ||
| 7397 | + ) | ||
| 7398 | + } | ||
| 7312 | "clear" if matches!(rt, Some(Nim::Seq(_)) | Some(Nim::Prim(_))) => { | 7399 | "clear" if matches!(rt, Some(Nim::Seq(_)) | Some(Nim::Prim(_))) => { |
| 7313 | (format!("{}.setLen(0)", recv.code), Some(Nim::Unit)) | 7400 | (format!("{}.setLen(0)", recv.code), Some(Nim::Unit)) |
| 7314 | } | 7401 | } |
modified
src/main.rs +3 -0 | @@ -71,6 +71,7 @@ fn run() -> Result<(), String> { | ||
| 71 | 71 | let mut cfgs: Vec<String> = Vec::new(); |
| 72 | 72 | let mut expand = false; |
| 73 | 73 | let mut prune = false; |
| 74 | + let mut report = false; | |
| 74 | 75 | |
| 75 | 76 | while let Some(a) = args.next() { |
| 76 | 77 | match a.to_string_lossy().as_ref() { |
| @@ -83,6 +84,7 @@ fn run() -> Result<(), String> { | ||
| 83 | 84 | } |
| 84 | 85 | "--expand" => expand = true, |
| 85 | 86 | "--prune" => prune = true, |
| 87 | + "--report" => report = true, | |
| 86 | 88 | "--cfg" => { |
| 87 | 89 | let v = args.next().ok_or("`--cfg` needs an argument")?; |
| 88 | 90 | let v = v.to_string_lossy().into_owned(); |
| @@ -175,6 +177,7 @@ fn run() -> Result<(), String> { | ||
| 175 | 177 | } |
| 176 | 178 | |
| 177 | 179 | let mut lowerer = lower::Lowerer::new(); |
| 180 | + lowerer.report = report; | |
| 178 | 181 | lowerer.features = features; |
| 179 | 182 | lowerer.cfgs = cfgs; |
| 180 | 183 | // A `mod x;` is satisfied when x.rs is one of the inputs. |
| @@ -71,6 +71,7 @@ fn run() -> Result<(), String> { | |||
| 71 | let mut cfgs: Vec<String> = Vec::new(); | 71 | let mut cfgs: Vec<String> = Vec::new(); |
| 72 | let mut expand = false; | 72 | let mut expand = false; |
| 73 | let mut prune = false; | 73 | let mut prune = false; |
| 74 | + let mut report = false; | ||
| 74 | 75 | ||
| 75 | while let Some(a) = args.next() { | 76 | while let Some(a) = args.next() { |
| 76 | match a.to_string_lossy().as_ref() { | 77 | match a.to_string_lossy().as_ref() { |
| @@ -83,6 +84,7 @@ fn run() -> Result<(), String> { | |||
| 83 | } | 84 | } |
| 84 | "--expand" => expand = true, | 85 | "--expand" => expand = true, |
| 85 | "--prune" => prune = true, | 86 | "--prune" => prune = true, |
| 87 | + "--report" => report = true, | ||
| 86 | "--cfg" => { | 88 | "--cfg" => { |
| 87 | let v = args.next().ok_or("`--cfg` needs an argument")?; | 89 | let v = args.next().ok_or("`--cfg` needs an argument")?; |
| 88 | let v = v.to_string_lossy().into_owned(); | 90 | let v = v.to_string_lossy().into_owned(); |
| @@ -175,6 +177,7 @@ fn run() -> Result<(), String> { | |||
| 175 | } | 177 | } |
| 176 | 178 | ||
| 177 | let mut lowerer = lower::Lowerer::new(); | 179 | let mut lowerer = lower::Lowerer::new(); |
| 180 | + lowerer.report = report; | ||
| 178 | lowerer.features = features; | 181 | lowerer.features = features; |
| 179 | lowerer.cfgs = cfgs; | 182 | lowerer.cfgs = cfgs; |
| 180 | // A `mod x;` is satisfied when x.rs is one of the inputs. | 183 | // A `mod x;` is satisfied when x.rs is one of the inputs. |