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

Fix for issue #553. #554

Merged
merged 1 commit into from
Dec 16, 2013
Merged

Fix for issue #553. #554

merged 1 commit into from
Dec 16, 2013

Conversation

redstar
Copy link
Member

@redstar redstar commented Dec 13, 2013

Check if a static array is accessed with a known index. In this case the bounds check can be omitted.

A bit irritating is the comment:

// static arrays could get static checks for static indices
// but shouldn't since it might be generic code that's never executed

because I do not see where the static bounds and indices might be changed later.

@yebblies
Copy link

Maybe it's best to wait until yebblies/dmd@186acf4 gets merged into ldc.

@redstar
Copy link
Member Author

redstar commented Dec 13, 2013

The pull request would make life easier. DMD does not generate bounds checking code for static arrays and I try do to the same. :-)
Any chance that the pull request will be merged soon?

@dnadlinger
Copy link
Member

@redstar: Regarding your question about the comment, maybe DMD didn't implement the proper checks for static arrays in the frontend yet. And if int[3] a; a[4] = 1; wasn't considered invalid code (but merely undefined behavior when executed), it would have been a mistake for LDC to error out on that.

@redstar
Copy link
Member Author

redstar commented Dec 14, 2013

@klickverbot Thanks for the explanation. I relaxed the check a bit. If the static indices are found to be outside of the static array, a warning is emitted and a runtime check is generated.

// If this happens then it is possible a frontend bug.
// Just output a warning and continue generating a runtime check.
// This could be generic code which is never executed.
warning(loc, "Static array index out of bounds (should have been catch during semantic analysis)");
Copy link
Member

Choose a reason for hiding this comment

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

caught/detected

@yebblies
Copy link

Any chance that the pull request will be merged soon?

@redstar That pull request has already been merged into dmd, just not ldc.

Check if a static array is accessed with a known index. In this case the
bounds check can be omitted.
@redstar
Copy link
Member Author

redstar commented Dec 16, 2013

@yebblies Thanks for the pointer. I think I will create another dot-release of 0.12 therefore I like to have this fixed now. But for the next major release I will for sure use skipboundscheck!

@redstar redstar merged commit bac536a into ldc-developers:master Dec 16, 2013
@redstar redstar deleted the issue553 branch December 16, 2013 15:28
redstar pushed a commit that referenced this pull request Sep 27, 2014
issue 10720 - ICE with is(aaOfNonCopyableStruct.nonExistingField)
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.

3 participants