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

Add error tests for mismatch in empty dtype #2307

Merged
merged 2 commits into from
Aug 28, 2023

Conversation

Shaikh-Ubaid
Copy link
Collaborator

Copy link
Contributor

@certik certik left a comment

Choose a reason for hiding this comment

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

I think this looks great.

I found a minor issue in the way the dimensions are printed. This can be fixed in subsequent PRs.

--> tests/errors/arrays_05.py:6:5
|
6 | x: i16[5, 4] = empty([5, 3], dtype=int16)
| ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^ type mismatch ('i16[5][4]' and 'i16[5][3]')
Copy link
Contributor

Choose a reason for hiding this comment

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

I would write it as i16[5, 4] as in the source code, not i16[5][4].

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you also suggest to include the start value of the dimension? For example i16[0:5, 0:4]? I guess it is only helpful for the developers. For users it might confuse as python does not have lower_bounds. (Although it could be helpful in the case for LFortran if we reuse this printing mechanism/function there.)

We can also include the start value of the dimension later (when we have more experience with error messages or user feedback).

Copy link
Contributor

Choose a reason for hiding this comment

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

For Python I would print it as just i16[5, 4]. For LFortran we use a different method to print the type, and there indeed we have an option to print the dimension in the usual Fortran notation --- depending on the context, sometimes the lower dimension matters, sometimes it doesn't.

@certik certik merged commit c1e8486 into lcompilers:main Aug 28, 2023
12 checks passed
@Shaikh-Ubaid Shaikh-Ubaid deleted the test_for_mismatch_in_empty_dtype branch August 28, 2023 22:17
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