Fix issue 22198 - Compile time bounds checking for static arrays

[Mai-Lapyst]

This PR adds checks to report at compile time when a slice expression onto a array which size is known at compiletime is out of bounds. It does that by checking for those inside the constantfolding code (compiler/src/dmd/constfold.d), where also errors for out-of-bounds index access for static arrays is reported.

Adding this causes a minor issue with __ArrayDtor, since it tries to use the slice syntax to lineralize multidimensional static arrays; this is fixed by casting it before the slice to its lineralized form; e.g. __ArrayDtor((cast(T[n])v)[0 .. n]).

[dlang-bot]

Thanks for your pull request and interest in making D better, @Mai-Lapyst! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	22198	enhancement	Compile time bounds checking for static arrays

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#15650"

[RazvanN7]

Why is the constant folder the component that issues these errors? This seems icky to me. The errors should be issued after the constant folder has done his job, somewhere in the innards of SliceExp semantic after constat folding has been done on the slice indexes.

[Mai-Lapyst]

Why is the constant folder the component that issues these errors? This seems icky to me. The errors should be issued after the constant folder has done his job, somewhere in the innards of SliceExp semantic after constat folding has been done on the slice indexes.

It is a bit controvercial, but the bound checks for IndexExp are also happening inside the constant folder:

https://github.com/dlang/dmd/blob/99ce1af0bb1431265cf7e5edcd9c3db8c79040b8/compiler/src/dmd/constfold.d#L1154

https://github.com/dlang/dmd/blob/99ce1af0bb1431265cf7e5edcd9c3db8c79040b8/compiler/src/dmd/constfold.d#L1177

(... and some more). So I think when the check + error for SliceExp should be generated at another location, IndexExp should as well; otherwise it would be even more complicated to reason about where which error comes from when debugging the compiler itself, since then similar components (IndexExp and SliceExp) generate errors in completly different parts of the compiler.

[Imperatorn]

@Mai-Lapyst

src/dmd/constfold.d(1283): Deprecation: argument (*es1.elements).length for format specification "%llu" must be ulong, not uint

[Mai-Lapyst]

src/dmd/constfold.d(1283): Deprecation: argument (*es1.elements).length for format specification "%llu" must be ulong, not uint

Thanks for the info; pushed a commit that hopefully fixes this.

[RazvanN7]

So I think when the check + error for SliceExp should be generated at another location, IndexExp should as well; otherwise it would be even more complicated to reason about where which error comes from when debugging the compiler itself, since then similar components (IndexExp and SliceExp) generate errors in completly different parts of the compiler.

I agree that both SliceExp and IndexExp should follow the same strategy. As things stand I cannot approve this PR which pushes things in the wrong direction. The correct course of action is to fix both SliceExp and IndexExp to issue errors after the optimizer has reduced the bounds expressions.

[Mai-Lapyst]

I agree that both SliceExp and IndexExp should follow the same strategy. As things stand I cannot approve this PR which pushes things in the wrong direction. The correct course of action is to fix both SliceExp and IndexExp to issue errors after the optimizer has reduced the bounds expressions.

@RazvanN7 Would be happy to change the PR to fix this. If you could give me a little hint in which file/ class the neccessary changes need to be made, as I'm still very unfamiliar with the whole dmd codebase, It would be a huge help.