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

[NB] correctly lower array records #10686

Merged
merged 1 commit into from
May 12, 2023

Conversation

kabdelhak
Copy link
Contributor

No description provided.

@kabdelhak kabdelhak added the COMP/OMC/New Backend Issues and pull requests related to the new backend. label May 12, 2023
@kabdelhak kabdelhak self-assigned this May 12, 2023
@kabdelhak kabdelhak enabled auto-merge (squash) May 12, 2023 09:31
algorithm
isComplex := match ty
case ARRAY() guard(isComplex(ty.elementType)) then true;
case ARRAY() then isComplexArray(ty.elementType);
Copy link
Member

Choose a reason for hiding this comment

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

The elementType should never be an array, in that case something has gone very wrong at least as far as the frontend is concerned. So unless the backend is doing something weird with arrays I would suggest removing this case, to avoid confusion about how arrays are represented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay, i was not sure about this. so the only special case i have to check is Array(Complex) basically?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will be removed by #10690

Copy link
Member

Choose a reason for hiding this comment

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

Yes, array types are never nested. More correct would be to use:

case ARRAY() then isComplex(ty.elementType);
case CONDITIONAL_ARRAY() then isComplexArray(ty.trueType);

CONDITIONAL_ARRAY is only used by the frontend to deal with if-expressions where the two branches have different dimensions, but it's nice to take that into account so the function works correctly for the frontend too.

@kabdelhak kabdelhak merged commit 9d08b82 into OpenModelica:master May 12, 2023
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
COMP/OMC/New Backend Issues and pull requests related to the new backend.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants