Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define what a "Primitive Type" is in Cadence #2929

Closed
dsainati1 opened this issue Nov 7, 2023 · 11 comments · Fixed by #2980
Closed

Define what a "Primitive Type" is in Cadence #2929

dsainati1 opened this issue Nov 7, 2023 · 11 comments · Fixed by #2980
Assignees

Comments

@dsainati1
Copy link
Contributor

Issue to be solved

Some logic (like in #2928) relies on a soft definition of certain types (e.g. string, int, bool) in Cadence being "primitive", as opposed to a complex type like an array, dictionary or composite. We should concretize this by specifying exactly which types are primitive vs concrete.

Suggested Solution

No response

@j1010001
Copy link
Member

@darkdrag00nv2 this is probably required for #2803

@darkdrag00nv2
Copy link
Contributor

@j1010001 yeah, wondering if we need to limit the function on just primitive/containers of primitives though. Asked on the PR: #2945 (comment)

@darkdrag00nv2
Copy link
Contributor

@turbolent @SupunS Any idea on how we want to approach defining a "Primitive" in Cadence? Like should this be a type, may be a subtype of AnyStruct or could it just be a constant array of types that we define without exposing to end users?

@SupunS
Copy link
Member

SupunS commented Nov 21, 2023

I guess the requirement here is mostly for the implementation of the types, correct? i.e: an implementation detail rather than a user-facing separation of types. In that case, a simple function on sema.Type interface should be sufficient.

Having said that, what is the separation that is being required here? is it just container (arrays, dictionaries, composite) types vs non-container types? In the stable-cadence branch, I've introduced the same functionality by means of of function ContainFieldsOrElements on the sema.Type:

// ContainFieldsOrElements returns true if value of the type can have nested values (fields or elements).
// This notion is to indicate that a type can be used to access its nested values using
// either index-expression or member-expression. e.g. `foo.bar` or `foo[bar]`.
// This is used to determine if a field/element of this type should be returning a reference or not.
//
// Only a subset of types has this characteristic. e.g:
// - Composites
// - Interfaces
// - Arrays (Variable/Constant sized)
// - Dictionaries
// - Restricted types
// - Optionals of the above.
// - Then there are also built-in simple types, like StorageCapabilityControllerType, BlockType, etc.
// where the type is implemented as a simple type, but they also have fields.
//
// This is different from the existing `ValueIndexableType` in the sense that it is also implemented by simple types
// but not all simple types are indexable.
// On the other-hand, some indexable types (e.g. String) shouldn't be treated/returned as references.
//
ContainFieldsOrElements() bool

The original purpose of that method was to determine if a reference should be returned during member access, but if the 'technical' requirement is the same, then we can re-purpose/re-name it to something more generic

@SupunS
Copy link
Member

SupunS commented Nov 21, 2023

The PRs/Issues linked (#2928 and #2803) are against the stable-cadence branch, so using the existing ContainFieldsOrElements method should be possible even now.

@SupunS
Copy link
Member

SupunS commented Nov 21, 2023

I think in our documentation we never defined or used the term 'primitive type' so far (the member access semantics use the term 'container type' vs 'non-container-type'). If we are going to use that technical grouping in multiple features, maybe it's a good idea to formalize/standardize this grouping (so the terminology is consistent across features/documentation) and also add it to the documentation as well.

@darkdrag00nv2
Copy link
Contributor

Having said that, what is the separation that is being required here? is it just container (arrays, dictionaries, composite) types vs non-container types?

@SupunS Yes, for the dereference feature, container vs non-container should be sufficient.

@darkdrag00nv2
Copy link
Contributor

Actually, there are two slightly different requirements.

#2928 - primitives (int, bool, string) VS complex (array, dictionary, composite)
#2803 - primitives + containers of primitives VS other complex types

So I guess we could define primitives as int, bool, string etc. and then #2803 will have to also include containers of primitives separately.

@dsainati1
Copy link
Contributor Author

Note that primitive types should also include Address and Path values (which function like numbers and strings, respectively)

@turbolent
Copy link
Member

@dsainati1 It should be possible now to update https://github.com/onflow/cadence/pull/2928/files#diff-c34bee2125d8f3673160cc7b3c912540ad811b4622a66c3a2ed6c25fc94e1cb0R2091 based on #2975

@dsainati1
Copy link
Contributor Author

#2980

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

Successfully merging a pull request may close this issue.

5 participants