Support size pragma with generic type parameters#25480
Open
elijahr wants to merge 4 commits intonim-lang:develfrom
Open
Support size pragma with generic type parameters#25480elijahr wants to merge 4 commits intonim-lang:develfrom
elijahr wants to merge 4 commits intonim-lang:develfrom
Conversation
Allows imported types to use `size: sizeof(T)` where T is a generic
type parameter, with evaluation deferred until instantiation.
Example:
type Atomic[T] {.importcpp: "std::atomic", size: sizeof(T).} = object
When instantiated as Atomic[int64], the size pragma evaluates to 8.
Implementation uses a sparse side-table in ModuleGraph rather than
adding fields to PType/PSym, avoiding per-type memory overhead.
Only types with deferred pragmas consume storage.
Includes NIF serialization support for incremental compilation.
Member
|
Well the CI is red. |
The containsGenericParam function was incorrectly treating any unresolved identifier (nkIdent) as a potential generic parameter. This caused sizeof(cint) on non-imported enums to fail with 'deferred size expressions only supported for imported types'. Fix by looking up identifiers in scope: - If unresolved: likely a generic param, defer evaluation - If resolves to skGenericParam: defer evaluation - If resolves to a known type (like cint): evaluate immediately Also fix substituteTypeParams to handle nkIdent nodes by matching against generic param names, and handle leaf nodes to avoid iterating over nodes without children. Fixes package CI failures in weave, sdl2_nim, constantine, etc.
Contributor
Author
|
@Araq Sorry about that, I have been swamped. I should have some time to address it over the next few days. |
When Atomic[T] is used inside a generic proc like store[T], the type instantiation creates Atomic[store.T] where T is still unresolved. The previous fix incorrectly treated nkIdent nodes (like 'sizeof' or 'int8') as unresolved generic types, causing deferred size evaluation to never complete. This fix: - Adds containsUnresolvedGenericType to check if substituted expression still has unresolved generic params (nkType with tyGenericParam) - Returns false for nkIdent nodes since they are regular identifiers that will be resolved during semantic analysis - Defers size evaluation only when expression truly contains unresolved generic types, allowing direct instantiations like Single[int16] to evaluate correctly
Contributor
Author
|
Okay, CI is green now |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allows imported types to use
size: sizeof(T)whereTis a generic type parameter, with evaluation deferred until instantiation.When instantiated as
Atomic[int64], the size pragma evaluates to 8.Implementation
typeExtensionsinModuleGraph) rather than adding fields toPType/PSym, avoiding per-type memory overhead.Changes
Supersedes #25403 per feedback from Araq.