Skip to content

Dyno: simplify array type creation #27177

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

Merged
merged 2 commits into from
Apr 29, 2025

Conversation

DanilaFe
Copy link
Contributor

@DanilaFe DanilaFe commented Apr 29, 2025

Closes https://github.com/Cray/chapel-private/issues/7334, which required resolving:

use ReplicatedDist;

var Space = {1..10};
const ReplicatedSpace = Space dmapped new replicatedDist();
var RA: [ReplicatedSpace] int;

To be merged after #27171. Clean diff: https://github.com/chapel-lang/chapel/pull/27177/files/47946ac16e576a8d92863f8cffb4efebccb2ff96..132e5f2c0931cfbd96003239e9499ab435abbbd3

The dom and eltType fields are required as part of the DSI.

In practice, they are relied on in various method on the _array builtin record:

pragma "no copy return"
pragma "return not owned"
proc _dom do return _getDomain(_value.dom);

/* The type of elements contained in the array */
proc eltType type do return _value.eltType;

So, instead of special casing various well-known array classes to extract their dom and eltType fields, simply adjust the InitResolver code to search for these required fields until they are found.

While there, it seemed easy enough to add errors for when these things are missing.

Reviewed by @arifthpe -- thanks!

Testing

  • dyno tests
  • test/release/examples/primers/replicated.chpl resolves
  • paratest

@arifthpe arifthpe self-requested a review April 29, 2025 19:02
@DanilaFe DanilaFe requested a review from arifthpe April 29, 2025 20:41
The `dom` and `eltType` fields are [required as part of the DSI](https://chapel-lang.org/docs/technotes/dsi.html#id11).

In practice, they are relied on in various method on the `_array` builtin record:

https://github.com/chapel-lang/chapel/blob/47946ac16e576a8d92863f8cffb4efebccb2ff96/modules/internal/ChapelArray.chpl#L832-L834

https://github.com/chapel-lang/chapel/blob/47946ac16e576a8d92863f8cffb4efebccb2ff96/modules/internal/ChapelArray.chpl#L819-L820

So, instead of special casing various well-known array classes to extract
their `dom` and `eltType` fields, simply adjust the InitResolver code to
search for these required fields until they are found.

Signed-off-by: Danila Fedorin <[email protected]>
@DanilaFe DanilaFe force-pushed the simplify-array-type-creation branch 2 times, most recently from 32ae63e to 2536a02 Compare April 29, 2025 22:19
@DanilaFe
Copy link
Contributor Author

Older GCCs (7.5) don't accept the cleaner lambda-based approach, so I've backed it out after checking with @arifthpe.

@DanilaFe DanilaFe merged commit 5db4e97 into chapel-lang:main Apr 29, 2025
10 checks passed
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