Resolve `Self::CONST` in an array length, and emit assoc consts earlier
`[u8; Self::MAX_STR_LEN]` resolves against whichever impl is being walked, so the current `Self` is now part of what the const-name map depends on -- and part of its cache key, or the map would be stale for the next impl. That exposed an ordering problem of the usual kind: the associated const was written out while emitting the impl body, but a *forward declaration* can name one in an array length, and forwards come first. Associated consts are emitted in the types pass now, with a guard so the body does not write them again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
8e6b002 parent: abf2c13 modified
src/lower.rs +33 -7 | @@ -341,11 +341,14 @@ pub struct Lowerer { | ||
| 341 | 341 | /// turn into an error. |
| 342 | 342 | local_items: HashMap<String, String>, |
| 343 | 343 | emitted_local: std::collections::HashSet<String>, |
| 344 | + /// Associated consts already written out, so the types pass and the | |
| 345 | + /// impl body cannot emit the same one twice. | |
| 346 | + emitted_assoc: std::collections::HashSet<(String, String)>, | |
| 344 | 347 | /// Const generic parameters, by name, with their Nim type. Nim's `static` |
| 345 | 348 | /// parameter is the same thing: a generic parameter that is a value. |
| 346 | 349 | const_gens: HashMap<String, String>, |
| 347 | 350 | /// Combined table size when `CONST_NAMES` was last rebuilt. |
| 348 | - const_names_at: std::cell::Cell<Option<usize>>, | |
| 351 | + const_names_at: std::cell::RefCell<Option<(usize, String)>>, | |
| 349 | 352 | /// Bodies of functions whose return type is a return-position `impl |
| 350 | 353 | /// Trait`, keyed the same way as `fns`/`methods`, so `resolve_opaque` can |
| 351 | 354 | /// read the concrete type back off the body. |
| @@ -432,8 +435,9 @@ impl Lowerer { | ||
| 432 | 435 | uses_i128: std::cell::Cell::new(false), |
| 433 | 436 | local_items: HashMap::new(), |
| 434 | 437 | emitted_local: std::collections::HashSet::new(), |
| 438 | + emitted_assoc: std::collections::HashSet::new(), | |
| 435 | 439 | const_gens: HashMap::new(), |
| 436 | - const_names_at: std::cell::Cell::new(None), | |
| 440 | + const_names_at: std::cell::RefCell::new(None), | |
| 437 | 441 | opaque_fns: Vec::new(), |
| 438 | 442 | opaque_ret: HashMap::new(), |
| 439 | 443 | opaque_hint: None, |
| @@ -1285,11 +1289,14 @@ impl Lowerer { | ||
| 1285 | 1289 | // their combined size is enough to tell whether anything has been |
| 1286 | 1290 | // added since the last refill -- without it, a 357-crate run spends |
| 1287 | 1291 | // essentially all of its time rebuilding this map. |
| 1292 | + // `Self::CONST` resolves against whichever impl is being walked, so | |
| 1293 | + // the current `Self` is part of what the map depends on. | |
| 1294 | + let me = self.self_ty.as_ref().map(type_name).unwrap_or_default(); | |
| 1288 | 1295 | let size = self.enums.len() + self.assoc_consts.len(); |
| 1289 | - if self.const_names_at.get() == Some(size) { | |
| 1296 | + if self.const_names_at.borrow().as_ref() == Some(&(size, me.clone())) { | |
| 1290 | 1297 | return; |
| 1291 | 1298 | } |
| 1292 | - self.const_names_at.set(Some(size)); | |
| 1299 | + *self.const_names_at.borrow_mut() = Some((size, me.clone())); | |
| 1293 | 1300 | ty::CONST_NAMES.with(|m| { |
| 1294 | 1301 | let mut m = m.borrow_mut(); |
| 1295 | 1302 | m.clear(); |
| @@ -1303,6 +1310,9 @@ impl Lowerer { | ||
| 1303 | 1310 | } |
| 1304 | 1311 | for ((owner, name), (nim, _)) in &self.assoc_consts { |
| 1305 | 1312 | m.insert(format!("{owner}::{name}"), nim.clone()); |
| 1313 | + if !me.is_empty() && owner == &me { | |
| 1314 | + m.insert(format!("Self::{name}"), nim.clone()); | |
| 1315 | + } | |
| 1306 | 1316 | } |
| 1307 | 1317 | }); |
| 1308 | 1318 | } |
| @@ -2082,12 +2092,22 @@ impl Lowerer { | ||
| 2082 | 2092 | // declared in this pass with the rest of them, not in the body. |
| 2083 | 2093 | Item::Fn(f) => self.body_types(&f.block), |
| 2084 | 2094 | Item::Impl(im) => { |
| 2095 | + // Associated consts belong to this pass: a forward | |
| 2096 | + // declaration can name one in an array length, and forwards | |
| 2097 | + // are written before any impl body is. | |
| 2098 | + let self_ty = self.map_ty(&im.self_ty)?; | |
| 2099 | + let outer = self.self_ty.replace(self_ty.clone()); | |
| 2100 | + let tyname = type_name(&self_ty); | |
| 2101 | + let mut r = Ok(()); | |
| 2085 | 2102 | for it in &im.items { |
| 2086 | - if let syn::ImplItem::Fn(m) = it { | |
| 2087 | - self.body_types(&m.block)?; | |
| 2103 | + match it { | |
| 2104 | + syn::ImplItem::Fn(m) => r = r.and(self.body_types(&m.block)), | |
| 2105 | + syn::ImplItem::Const(c) => r = r.and(self.assoc_const(&tyname, c)), | |
| 2106 | + _ => {} | |
| 2088 | 2107 | } |
| 2089 | 2108 | } |
| 2090 | - Ok(()) | |
| 2109 | + self.self_ty = outer; | |
| 2110 | + r | |
| 2091 | 2111 | } |
| 2092 | 2112 | Item::Mod(m) if m.content.is_some() => { |
| 2093 | 2113 | let items = m.content.as_ref().map(|(_, i)| i.clone()).unwrap_or_default(); |
| @@ -2447,6 +2467,12 @@ impl Lowerer { | ||
| 2447 | 2467 | /// `const N: usize = 4;` inside an `impl`. Nim has no per-type constant |
| 2448 | 2468 | /// namespace, so it becomes a module-level const named for both. |
| 2449 | 2469 | fn assoc_const(&mut self, tyname: &str, c: &syn::ImplItemConst) -> Result<(), String> { |
| 2470 | + if !self | |
| 2471 | + .emitted_assoc | |
| 2472 | + .insert((tyname.to_string(), c.ident.to_string())) | |
| 2473 | + { | |
| 2474 | + return Ok(()); | |
| 2475 | + } | |
| 2450 | 2476 | let t = self.map_ty(&c.ty)?.owned(); |
| 2451 | 2477 | let v = self.expr_at(&c.expr, Some(&t))?; |
| 2452 | 2478 | let name = format!("{}_{}", tyname, c.ident); |
| @@ -341,11 +341,14 @@ pub struct Lowerer { | |||
| 341 | /// turn into an error. | 341 | /// turn into an error. |
| 342 | local_items: HashMap<String, String>, | 342 | local_items: HashMap<String, String>, |
| 343 | emitted_local: std::collections::HashSet<String>, | 343 | emitted_local: std::collections::HashSet<String>, |
| 344 | + /// Associated consts already written out, so the types pass and the | ||
| 345 | + /// impl body cannot emit the same one twice. | ||
| 346 | + emitted_assoc: std::collections::HashSet<(String, String)>, | ||
| 344 | /// Const generic parameters, by name, with their Nim type. Nim's `static` | 347 | /// Const generic parameters, by name, with their Nim type. Nim's `static` |
| 345 | /// parameter is the same thing: a generic parameter that is a value. | 348 | /// parameter is the same thing: a generic parameter that is a value. |
| 346 | const_gens: HashMap<String, String>, | 349 | const_gens: HashMap<String, String>, |
| 347 | /// Combined table size when `CONST_NAMES` was last rebuilt. | 350 | /// Combined table size when `CONST_NAMES` was last rebuilt. |
| 348 | - const_names_at: std::cell::Cell<Option<usize>>, | 351 | + const_names_at: std::cell::RefCell<Option<(usize, String)>>, |
| 349 | /// Bodies of functions whose return type is a return-position `impl | 352 | /// Bodies of functions whose return type is a return-position `impl |
| 350 | /// Trait`, keyed the same way as `fns`/`methods`, so `resolve_opaque` can | 353 | /// Trait`, keyed the same way as `fns`/`methods`, so `resolve_opaque` can |
| 351 | /// read the concrete type back off the body. | 354 | /// read the concrete type back off the body. |
| @@ -432,8 +435,9 @@ impl Lowerer { | |||
| 432 | uses_i128: std::cell::Cell::new(false), | 435 | uses_i128: std::cell::Cell::new(false), |
| 433 | local_items: HashMap::new(), | 436 | local_items: HashMap::new(), |
| 434 | emitted_local: std::collections::HashSet::new(), | 437 | emitted_local: std::collections::HashSet::new(), |
| 438 | + emitted_assoc: std::collections::HashSet::new(), | ||
| 435 | const_gens: HashMap::new(), | 439 | const_gens: HashMap::new(), |
| 436 | - const_names_at: std::cell::Cell::new(None), | 440 | + const_names_at: std::cell::RefCell::new(None), |
| 437 | opaque_fns: Vec::new(), | 441 | opaque_fns: Vec::new(), |
| 438 | opaque_ret: HashMap::new(), | 442 | opaque_ret: HashMap::new(), |
| 439 | opaque_hint: None, | 443 | opaque_hint: None, |
| @@ -1285,11 +1289,14 @@ impl Lowerer { | |||
| 1285 | // their combined size is enough to tell whether anything has been | 1289 | // their combined size is enough to tell whether anything has been |
| 1286 | // added since the last refill -- without it, a 357-crate run spends | 1290 | // added since the last refill -- without it, a 357-crate run spends |
| 1287 | // essentially all of its time rebuilding this map. | 1291 | // essentially all of its time rebuilding this map. |
| 1292 | + // `Self::CONST` resolves against whichever impl is being walked, so | ||
| 1293 | + // the current `Self` is part of what the map depends on. | ||
| 1294 | + let me = self.self_ty.as_ref().map(type_name).unwrap_or_default(); | ||
| 1288 | let size = self.enums.len() + self.assoc_consts.len(); | 1295 | let size = self.enums.len() + self.assoc_consts.len(); |
| 1289 | - if self.const_names_at.get() == Some(size) { | 1296 | + if self.const_names_at.borrow().as_ref() == Some(&(size, me.clone())) { |
| 1290 | return; | 1297 | return; |
| 1291 | } | 1298 | } |
| 1292 | - self.const_names_at.set(Some(size)); | 1299 | + *self.const_names_at.borrow_mut() = Some((size, me.clone())); |
| 1293 | ty::CONST_NAMES.with(|m| { | 1300 | ty::CONST_NAMES.with(|m| { |
| 1294 | let mut m = m.borrow_mut(); | 1301 | let mut m = m.borrow_mut(); |
| 1295 | m.clear(); | 1302 | m.clear(); |
| @@ -1303,6 +1310,9 @@ impl Lowerer { | |||
| 1303 | } | 1310 | } |
| 1304 | for ((owner, name), (nim, _)) in &self.assoc_consts { | 1311 | for ((owner, name), (nim, _)) in &self.assoc_consts { |
| 1305 | m.insert(format!("{owner}::{name}"), nim.clone()); | 1312 | m.insert(format!("{owner}::{name}"), nim.clone()); |
| 1313 | + if !me.is_empty() && owner == &me { | ||
| 1314 | + m.insert(format!("Self::{name}"), nim.clone()); | ||
| 1315 | + } | ||
| 1306 | } | 1316 | } |
| 1307 | }); | 1317 | }); |
| 1308 | } | 1318 | } |
| @@ -2082,12 +2092,22 @@ impl Lowerer { | |||
| 2082 | // declared in this pass with the rest of them, not in the body. | 2092 | // declared in this pass with the rest of them, not in the body. |
| 2083 | Item::Fn(f) => self.body_types(&f.block), | 2093 | Item::Fn(f) => self.body_types(&f.block), |
| 2084 | Item::Impl(im) => { | 2094 | Item::Impl(im) => { |
| 2095 | + // Associated consts belong to this pass: a forward | ||
| 2096 | + // declaration can name one in an array length, and forwards | ||
| 2097 | + // are written before any impl body is. | ||
| 2098 | + let self_ty = self.map_ty(&im.self_ty)?; | ||
| 2099 | + let outer = self.self_ty.replace(self_ty.clone()); | ||
| 2100 | + let tyname = type_name(&self_ty); | ||
| 2101 | + let mut r = Ok(()); | ||
| 2085 | for it in &im.items { | 2102 | for it in &im.items { |
| 2086 | - if let syn::ImplItem::Fn(m) = it { | 2103 | + match it { |
| 2087 | - self.body_types(&m.block)?; | 2104 | + syn::ImplItem::Fn(m) => r = r.and(self.body_types(&m.block)), |
| 2105 | + syn::ImplItem::Const(c) => r = r.and(self.assoc_const(&tyname, c)), | ||
| 2106 | + _ => {} | ||
| 2088 | } | 2107 | } |
| 2089 | } | 2108 | } |
| 2090 | - Ok(()) | 2109 | + self.self_ty = outer; |
| 2110 | + r | ||
| 2091 | } | 2111 | } |
| 2092 | Item::Mod(m) if m.content.is_some() => { | 2112 | Item::Mod(m) if m.content.is_some() => { |
| 2093 | let items = m.content.as_ref().map(|(_, i)| i.clone()).unwrap_or_default(); | 2113 | let items = m.content.as_ref().map(|(_, i)| i.clone()).unwrap_or_default(); |
| @@ -2447,6 +2467,12 @@ impl Lowerer { | |||
| 2447 | /// `const N: usize = 4;` inside an `impl`. Nim has no per-type constant | 2467 | /// `const N: usize = 4;` inside an `impl`. Nim has no per-type constant |
| 2448 | /// namespace, so it becomes a module-level const named for both. | 2468 | /// namespace, so it becomes a module-level const named for both. |
| 2449 | fn assoc_const(&mut self, tyname: &str, c: &syn::ImplItemConst) -> Result<(), String> { | 2469 | fn assoc_const(&mut self, tyname: &str, c: &syn::ImplItemConst) -> Result<(), String> { |
| 2470 | + if !self | ||
| 2471 | + .emitted_assoc | ||
| 2472 | + .insert((tyname.to_string(), c.ident.to_string())) | ||
| 2473 | + { | ||
| 2474 | + return Ok(()); | ||
| 2475 | + } | ||
| 2450 | let t = self.map_ty(&c.ty)?.owned(); | 2476 | let t = self.map_ty(&c.ty)?.owned(); |
| 2451 | let v = self.expr_at(&c.expr, Some(&t))?; | 2477 | let v = self.expr_at(&c.expr, Some(&t))?; |
| 2452 | let name = format!("{}_{}", tyname, c.ident); | 2478 | let name = format!("{}_{}", tyname, c.ident); |
modified
tests/cases/052-const-array-len.rs +8 -0 | @@ -16,6 +16,11 @@ struct Oid; | ||
| 16 | 16 | |
| 17 | 17 | impl Oid { |
| 18 | 18 | const MAX_SIZE: usize = 5; |
| 19 | + | |
| 20 | + // `Self::` in a length, resolving against the impl being walked. | |
| 21 | + fn buffer() -> [u8; Self::MAX_SIZE] { | |
| 22 | + [9u8; Self::MAX_SIZE] | |
| 23 | + } | |
| 19 | 24 | } |
| 20 | 25 | |
| 21 | 26 | struct Buf { |
| @@ -51,6 +56,9 @@ fn main() { | ||
| 51 | 56 | |
| 52 | 57 | // An associated const as a length, and in the braces rustc's expanded |
| 53 | 58 | // output writes it with. |
| 59 | + let b2 = Oid::buffer(); | |
| 60 | + println!("{} {}", b2.len(), sum(&b2)); | |
| 61 | + | |
| 54 | 62 | let q: [u8; { Oid::MAX_SIZE }] = [2u8; Oid::MAX_SIZE]; |
| 55 | 63 | println!("{} {}", q.len(), sum(&q)); |
| 56 | 64 | |
| @@ -16,6 +16,11 @@ struct Oid; | |||
| 16 | 16 | ||
| 17 | impl Oid { | 17 | impl Oid { |
| 18 | const MAX_SIZE: usize = 5; | 18 | const MAX_SIZE: usize = 5; |
| 19 | + | ||
| 20 | + // `Self::` in a length, resolving against the impl being walked. | ||
| 21 | + fn buffer() -> [u8; Self::MAX_SIZE] { | ||
| 22 | + [9u8; Self::MAX_SIZE] | ||
| 23 | + } | ||
| 19 | } | 24 | } |
| 20 | 25 | ||
| 21 | struct Buf { | 26 | struct Buf { |
| @@ -51,6 +56,9 @@ fn main() { | |||
| 51 | 56 | ||
| 52 | // An associated const as a length, and in the braces rustc's expanded | 57 | // An associated const as a length, and in the braces rustc's expanded |
| 53 | // output writes it with. | 58 | // output writes it with. |
| 59 | + let b2 = Oid::buffer(); | ||
| 60 | + println!("{} {}", b2.len(), sum(&b2)); | ||
| 61 | + | ||
| 54 | let q: [u8; { Oid::MAX_SIZE }] = [2u8; Oid::MAX_SIZE]; | 62 | let q: [u8; { Oid::MAX_SIZE }] = [2u8; Oid::MAX_SIZE]; |
| 55 | println!("{} {}", q.len(), sum(&q)); | 63 | println!("{} {}", q.len(), sum(&q)); |
| 56 | 64 | ||