nandi/rustnim · cb19206

⑂ cb19206

Stop dropping the cast in an array length, and resolve qualified names

`Expr::Cast` was folded by discarding the cast, which is wrong twice over:
`false as usize` is 0, and `PropertyId::Unset as usize` is an ordinal, and
neither is what the operand renders to on its own. That is how a `false`
turned up as an array length. Nim's `ord` covers all three cases exactly as
Rust's `as` does here -- identity on an integer, 0/1 on a bool, the ordinal on
an enum -- so that is what it lowers to now.

The qualified-path case was also a guess: `E::V` and `T::C` are spelled
identically, and the previous commit rendered both as an associated const.
They now come from the lowering's own tables, and a name in neither is an
error rather than an invented identifier.

Reaching those tables from `ty.rs` goes through a thread-local, refilled
before each mapping. That is a side channel and worth naming as one: `ty::map`
recurses through seventeen sites with no use for a context, and threading one
through all of them to serve array lengths alone is the worse trade.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

nandithebull committed 2026-09-19T10:36:47-07:00 Browse files

cb19206 parent: 04f5b3e

modified src/lower.rs +21 -0 Viewed

@@ -1177,6 +1177,7 @@ impl Lowerer {
1177	1177	/// lowering goes through here rather than calling `ty::map` directly, so
1178	1178	/// an alias cannot be missed in one position and honoured in another.
1179	1179	fn map_ty(&self, t: &syn::Type) -> Result<Nim, String> {
	1180	+ self.refill_const_names();
1180	1181	let n = self.map_ty_inner(t)?;
1181	1182	if mentions_sync(&n) {
1182	1183	self.uses_sync.set(true);
@@ -1184,6 +1185,26 @@ impl Lowerer {
1184	1185	Ok(n)
1185	1186	}
1186	1187
	1188	+ /// Publish the `Qualifier::NAME` constants an array length may refer to.
	1189	+ /// See `ty::CONST_NAMES` for why this is a side channel.
	1190	+ fn refill_const_names(&self) {
	1191	+ ty::CONST_NAMES.with(\|m\| {
	1192	+ let mut m = m.borrow_mut();
	1193	+ m.clear();
	1194	+ for (owner, def) in &self.enums {
	1195	+ if !def.simple {
	1196	+ continue;
	1197	+ }
	1198	+ for v in &def.variants {
	1199	+ m.insert(format!("{}::{}", owner, v.name), ident(&v.name));
	1200	+ }
	1201	+ }
	1202	+ for ((owner, name), (nim, _)) in &self.assoc_consts {
	1203	+ m.insert(format!("{owner}::{name}"), nim.clone());
	1204	+ }
	1205	+ });
	1206	+ }
	1207	+
1187	1208	fn map_ty_inner(&self, t: &syn::Type) -> Result<Nim, String> {
1188	1209	// `Self::Item` names an associated type of the enclosing `impl`.
1189	1210	if let syn::Type::Path(p) = t {

      /// lowering goes through here rather than calling `ty::map` directly, so
      /// an alias cannot be missed in one position and honoured in another.
      fn map_ty(&self, t: &syn::Type) -> Result<Nim, String> {
++        self.refill_const_names();
          let n = self.map_ty_inner(t)?;
          if mentions_sync(&n) {
              self.uses_sync.set(true);
          Ok(n)
      }
++    /// Publish the `Qualifier::NAME` constants an array length may refer to.
++    /// See `ty::CONST_NAMES` for why this is a side channel.
++    fn refill_const_names(&self) {
++        ty::CONST_NAMES.with(|m| {
++            let mut m = m.borrow_mut();
++            m.clear();
++            for (owner, def) in &self.enums {
++                if !def.simple {
++                    continue;
++                }
++                for v in &def.variants {
++                    m.insert(format!("{}::{}", owner, v.name), ident(&v.name));
++                }
++            }
++            for ((owner, name), (nim, _)) in &self.assoc_consts {
++                m.insert(format!("{owner}::{name}"), nim.clone());
++            }
++        });
++    }
++
      fn map_ty_inner(&self, t: &syn::Type) -> Result<Nim, String> {
          // `Self::Item` names an associated type of the enclosing `impl`.
          if let syn::Type::Path(p) = t {

modified src/ty.rs +35 -2 Viewed

@@ -534,6 +534,22 @@ fn discriminant(t: &Type) -> &'static str {
534	534	/// bound by a `const`, and the four arithmetic operators mean the same thing
535	535	/// and are written the same way. Anything outside it -- a const fn call, say --
536	536	/// is reported rather than guessed at.
	537	+thread_local! {
	538	+ /// Nim names for `Qualifier::NAME` paths that can appear in a const
	539	+ /// expression -- enum variants and associated consts.
	540	+ ///
	541	+ /// A thread-local rather than a parameter because `map` recurses through
	542	+ /// seventeen call sites that have no use for it, and the alternative is
	543	+ /// threading a context through all of them to serve array lengths alone.
	544	+ /// The `Lowerer` refills it before every mapping; see `Lowerer::map_ty`.
	545	+ pub static CONST_NAMES: std::cell::RefCell<std::collections::HashMap<String, String>> =
	546	+ std::cell::RefCell::new(std::collections::HashMap::new());
	547	+}
	548	+
	549	+fn known_const(q: &str, n: &str) -> Option<String> {
	550	+ CONST_NAMES.with(\|m\| m.borrow().get(&format!("{q}::{n}")).cloned())
	551	+}
	552	+
537	553	pub fn const_expr(e: &syn::Expr) -> Result<String, String> {
538	554	use syn::{BinOp, Expr};
539	555	match e {
@@ -556,8 +572,18 @@ pub fn const_expr(e: &syn::Expr) -> Result<String, String> {
556	572	// under the flattened name `Type_CONST`. A lowercase qualifier is
557	573	// a module path instead, and carries no name of its own.
558	574	if let Some(q) = segs.iter().rev().nth(1) {
	575	+ // An enum variant or an associated const, whichever the
	576	+ // lowering actually has under that name. Guessing between them
	577	+ // from the spelling alone is not possible -- `E::V` and
	578	+ // `T::C` look identical -- so the answer comes from the tables.
	579	+ if let Some(n) = known_const(q, last) {
	580	+ return Ok(n);
	581	+ }
559	582	if q.starts_with(char::is_uppercase) {
560		- return Ok(crate::lower::ident(&format!("{q}_{last}")));
	583	+ return Err(format!(
	584	+ "array length names `{q}::{last}`, which is neither an \
	585	+ enum variant nor an associated const rustnim has seen"
	586	+ ));
561	587	}
562	588	}
563	589	Ok(last.clone())
@@ -628,7 +654,14 @@ pub fn const_expr(e: &syn::Expr) -> Result<String, String> {
628	654	Expr::MethodCall(m) if m.method == "len" && m.args.is_empty() => {
629	655	Ok(format!("({}).len", const_expr(&m.receiver)?))
630	656	}
631		- Expr::Cast(c) => const_expr(&c.expr),
	657	+ // `X as usize`. Stripping the cast would be wrong: `false as usize` is
	658	+ // 0, and `PropertyId::Unset as usize` is an ordinal, neither of which
	659	+ // is what the operand renders to on its own. Nim's `ord` is the one
	660	+ // spelling that covers all three -- identity on an integer, 0/1 on a
	661	+ // bool, the ordinal on an enum -- and is exactly what Rust's `as` does
	662	+ // here.
	663	+ Expr::Cast(c) => Ok(format!("ord({})", const_expr(&c.expr)?)),
	664	+ Expr::Lit(syn::ExprLit { lit: syn::Lit::Bool(b), .. }) => Ok(b.value().to_string()),
632	665	other => Err(format!(
633	666	"array length is not a const expression rustnim can render: `{}`",
634	667	quote::quote!(#other)

  /// bound by a `const`, and the four arithmetic operators mean the same thing
  /// and are written the same way. Anything outside it -- a const fn call, say --
  /// is reported rather than guessed at.
++thread_local! {
++    /// Nim names for `Qualifier::NAME` paths that can appear in a const
++    /// expression -- enum variants and associated consts.
++    ///
++    /// A thread-local rather than a parameter because `map` recurses through
++    /// seventeen call sites that have no use for it, and the alternative is
++    /// threading a context through all of them to serve array lengths alone.
++    /// The `Lowerer` refills it before every mapping; see `Lowerer::map_ty`.
++    pub static CONST_NAMES: std::cell::RefCell<std::collections::HashMap<String, String>> =
++        std::cell::RefCell::new(std::collections::HashMap::new());
++}
++
++fn known_const(q: &str, n: &str) -> Option<String> {
++    CONST_NAMES.with(|m| m.borrow().get(&format!("{q}::{n}")).cloned())
++}
++
  pub fn const_expr(e: &syn::Expr) -> Result<String, String> {
      use syn::{BinOp, Expr};
      match e {
              // under the flattened name `Type_CONST`. A lowercase qualifier is
              // a module path instead, and carries no name of its own.
              if let Some(q) = segs.iter().rev().nth(1) {
++                // An enum variant or an associated const, whichever the
++                // lowering actually has under that name. Guessing between them
++                // from the spelling alone is not possible -- `E::V` and
++                // `T::C` look identical -- so the answer comes from the tables.
++                if let Some(n) = known_const(q, last) {
++                    return Ok(n);
++                }
                  if q.starts_with(char::is_uppercase) {
--                    return Ok(crate::lower::ident(&format!("{q}_{last}")));
++                    return Err(format!(
++                        "array length names `{q}::{last}`, which is neither an \
++                         enum variant nor an associated const rustnim has seen"
++                    ));
                  }
              }
              Ok(last.clone())
          Expr::MethodCall(m) if m.method == "len" && m.args.is_empty() => {
              Ok(format!("({}).len", const_expr(&m.receiver)?))
          }
--        Expr::Cast(c) => const_expr(&c.expr),
++        // `X as usize`. Stripping the cast would be wrong: `false as usize` is
++        // 0, and `PropertyId::Unset as usize` is an ordinal, neither of which
++        // is what the operand renders to on its own. Nim's `ord` is the one
++        // spelling that covers all three -- identity on an integer, 0/1 on a
++        // bool, the ordinal on an enum -- and is exactly what Rust's `as` does
++        // here.
++        Expr::Cast(c) => Ok(format!("ord({})", const_expr(&c.expr)?)),
++        Expr::Lit(syn::ExprLit { lit: syn::Lit::Bool(b), .. }) => Ok(b.value().to_string()),
          other => Err(format!(
              "array length is not a const expression rustnim can render: `{}`",
              quote::quote!(#other)

modified tests/cases/052-const-array-len.rs +16 -0 Viewed

@@ -5,6 +5,13 @@ const BLOCK: usize = 4;
5	5	const DOUBLE: usize = BLOCK * 2;
6	6	const FEATURES: [u8; 3] = [1, 2, 3];
7	7
	8	+#[derive(Clone, Copy)]
	9	+enum Slot {
	10	+ First,
	11	+ Second,
	12	+ Limit,
	13	+}
	14	+
8	15	struct Oid;
9	16
10	17	impl Oid {
@@ -51,4 +58,13 @@ fn main() {
51	58	// type in both languages, so it is a constant on both sides.
52	59	let r: [u8; FEATURES.len()] = [4u8; FEATURES.len()];
53	60	println!("{} {}", r.len(), sum(&r));
	61	+
	62	+ // An enum variant cast to a length. Stripping the `as` would be wrong:
	63	+ // what is wanted is the ordinal, which is what Nim's `ord` gives.
	64	+ let e: [u8; Slot::Limit as usize] = [6u8; Slot::Limit as usize];
	65	+ println!("{} {}", e.len(), sum(&e));
	66	+
	67	+ // `false as usize` is 0 and `true as usize` is 1 — also `ord`.
	68	+ let t: [u8; true as usize] = [8u8; true as usize];
	69	+ println!("{} {}", t.len(), sum(&t));
54	70	}

  const DOUBLE: usize = BLOCK * 2;
  const FEATURES: [u8; 3] = [1, 2, 3];
++#[derive(Clone, Copy)]
++enum Slot {
++    First,
++    Second,
++    Limit,
++}
++
  struct Oid;
  impl Oid {
      // type in both languages, so it is a constant on both sides.
      let r: [u8; FEATURES.len()] = [4u8; FEATURES.len()];
      println!("{} {}", r.len(), sum(&r));
++
++    // An enum variant cast to a length. Stripping the `as` would be wrong:
++    // what is wanted is the ordinal, which is what Nim's `ord` gives.
++    let e: [u8; Slot::Limit as usize] = [6u8; Slot::Limit as usize];
++    println!("{} {}", e.len(), sum(&e));
++
++    // `false as usize` is 0 and `true as usize` is 1 — also `ord`.
++    let t: [u8; true as usize] = [8u8; true as usize];
++    println!("{} {}", t.len(), sum(&t));
  }