adds assert messages for container.array

[burner]

No description provided.

[dlang-bot]

Thanks for your pull request, @burner!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

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

dub fetch digger
dub run digger -- build "master + phobos#7190"

[jondegenhardt]

Thanks for doing this. These assert messages are really valuable, they have helped me many times.

The text of the assert messages could be improved by changing the text to be more consistent with the other Array asserts. This would also make them consistent with the most common assert message format used in Phobos range methods. Most valuable would be to include the data type in the assert message. In this case, "Array". Specific suggestions for the first few cases:

• moveFront:
  • "Attempting to moveFront an empty Array"
  • "Attempting to moveFront using an out of bounds low index on an Array"
• moveBack
  • "Attempting to moveBack an empty Array"
  • "Attempting to moveBack using an out of bounds high index on an Array"
• moveAt
  • "Attempting to moveAt using an out of bounds index on an Array"
• opIndex
  • "Attempting to fetch using an out of bounds index on an Array"

The style is taken from examples found by grep'ing Phobos source files for assert and Attempting.

There are many range assert messages that don't adhere to this style, but it appears to be the most common. A somewhat common difference occurs with opIndex style messages, many of which have formats <object-type> <operation> index out of bounds. Examples:

"chunks index out of bounds"
"slide slicing index out of bounds"

[jondegenhardt]

To clarify my previous comment - I'm fine with the PR being merged as is if the author doesn't want to invest additional time in further improvements to the text. This PR is a meaningful improvement over the current assert messages (none). I looked through them all and didn't see any where the text was inconsistent with the method.

A separate thing - I do wonder if the memory allocation failure cases should be asserts. Not sure what the standard mechanism is in Phobos. However, they are asserts currently, this PR is not attempting to look at that, just the assert message text.

[burner]

@jondegenhardt thank you for the review. I'll keep a look later this week.

[jondegenhardt]

Very nice. Thanks for making these updates.

[burner]

@jondegenhardt thanks for the review

[burner]

@n8sh @atilaneves @jondegenhardt thank you

[PetarKirov]

(Merging manually as buildkite failure is not related to this PR.)

[burner]

@PetarKirov thank you