[flang] Tests for rank-1 integer array bounds in explicit shape arrays#411
[flang] Tests for rank-1 integer array bounds in explicit shape arrays#411ivanrodriguez3753 wants to merge 1 commit into
Conversation
tarunprabhu
left a comment
tarunprabhu
left a comment
There was a problem hiding this comment.
I have requested reviews from folks who are more familiar with F2023 features.
| # Exclude tests that require unimplemented feature (conditional expression as | ||
| # actual argument) | ||
| file(GLOB _sources *.f90) | ||
|
|
||
| set(Source ${_sources}) |
There was a problem hiding this comment.
I am not sure what this is doing.
ivanrodriguez3753
force-pushed
the
int_array_explicit_shape
branch
from
bdefd19 to
24897f4
Compare
|
I will review this soon. It is in my queue. |
kwyatt-ext
left a comment
kwyatt-ext
left a comment
There was a problem hiding this comment.
It is a very well written test. 2 things to note:
- Recommended case addition.
- I didn't check but make sure code is merged before merging this.
| ! 10. Implied-do + array slices + arithmetic in bounds | ||
| ! 11. Vector subscripts in bounds expressions | ||
| ! 12. Block construct re-evaluation — bounds from enclosing scope | ||
|
|
There was a problem hiding this comment.
I recommend adding a case with a UB < LB. It should produce a valid array of 0 size, but could happen in dynamic code.
cenewcombe
left a comment
cenewcombe
left a comment
There was a problem hiding this comment.
I left an inline comment for you to consider. I don't need to re-review, looks good!
| ! 9. Element write/read correctness | ||
| ! 10. Implied-do + array slices + arithmetic in bounds | ||
| ! 11. Vector subscripts in bounds expressions | ||
| ! 12. Block construct re-evaluation — bounds from enclosing scope |
There was a problem hiding this comment.
A few more edge cases for your consideration:
- negative bounds
- modify/clobber the bounds variable within the same scope (verify that bounds do not change)
- declare a constant (PARAMETER) bounds array -- exercises the compile-time constant-folding path vs. the runtime extraction path
This comment is meant to be non-blocking. The test coverage is very good. It would also be reasonable to implement some of these checks as lit tests rather than in the test suite (or they may already be covered in lit).
5 participants
No description provided.