Skip to content

[DNM] Fix wide-string override_ty#1645

Draft
fw-immunant wants to merge 3 commits intomasterfrom
fw/arraytopointer-override-ty
Draft

[DNM] Fix wide-string override_ty#1645
fw-immunant wants to merge 3 commits intomasterfrom
fw/arraytopointer-override-ty

Conversation

@fw-immunant
Copy link
Contributor

This isn't suitable for merging, but shows the constraints we're working in--namely, we need to synthesize a new CTypeId (for the appropriately-sized array type of wide-chars, to replace the same array of c_ints) in a place where the TypedAstContext is not mutable.

…AstContext::c_types`

note: this is a hack because it adds a memory leak in the `Index` impl
@Rua
Copy link
Contributor

Rua commented Mar 6, 2026

I was thinking about this but had a different approach in mind. Instead of trying to synthesize a new type, just pass an override for the pointee/element type to convert_array_to_pointer_decay (and only there). For array decay it's always the same pattern between source and target types, where [foo; N] turns into *const/mut foo. It's not even technically a cast, anyway.

@fw-immunant
Copy link
Contributor Author

I was thinking about this but had a different approach in mind. Instead of trying to synthesize a new type, just pass an override for the pointee/element type to convert_array_to_pointer_decay (and only there). For array decay it's always the same pattern between source and target types, where [foo; N] turns into *const/mut foo. It's not even technically a cast, anyway.

I believe the problem with that approach is that the val argument to convert_array_to_pointer_decay has already been translated (its type is WithStmts<Box<Expr>>) with the wrong override_ty.

Of course, we could address that via inversion of control, changing convert_cast to be responsible for the translation of the value to cast.

@fw-immunant
Copy link
Contributor Author

This isn't suitable for merging, but shows the constraints we're working in--namely, we need to synthesize a new CTypeId (for the appropriately-sized array type of wide-chars, to replace the same array of c_ints) in a place where the TypedAstContext is not mutable.

One note - I used dashmap here in the hopes that it would let us insert new CTypeIds to the c_types map without disturbing the required Index impl which must return &CType, but since it doesn't actually do that (we have to leak a boxed CType to do so because DashMap returns its own Ref<V> type rather than &V), dashmap is not really helping compared to a RefCell<HashMap>, and what would actually solve the Index problem is an append-only, address-stable hashmap. That should fix the hackiness side of things here.

@fw-immunant fw-immunant force-pushed the fw/arraytopointer-override-ty branch from 9484b2a to 6e15fbf Compare March 6, 2026 21:12
};
//
// && Some(source_ty.ctype) != override_ty.map(|x| x.ctype)
let new_array_override_ty = self
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than trying to synthesize a completely new array type here, could convert_array_to_pointer_decay not be modified to take an element type instead of source and target types, and then called directly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants