Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Fix confusing message on array cast failure. #3792

Merged
merged 1 commit into from
Mar 31, 2022

Conversation

schveiguy
Copy link
Member

Still needs a bug report. First I want to see it pass CI, as I'm not sure if there's a test somewhere for this message.

@dlang-bot
Copy link
Contributor

dlang-bot commented Mar 31, 2022

Thanks for your pull request, @schveiguy!

Bugzilla references

Auto-close Bugzilla Severity Description
22964 enhancement array cast message is awkwardly worded

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 + druntime#3792"

@schveiguy schveiguy force-pushed the fixarraycast branch 2 times, most recently from 72becbe to 6bde6d3 Compare March 31, 2022 14:38
@dlang-bot dlang-bot added the Enhancement New functionality label Mar 31, 2022
@schveiguy schveiguy marked this pull request as ready for review March 31, 2022 14:39
@schveiguy schveiguy requested a review from MartinNowak as a code owner March 31, 2022 14:39
@schveiguy
Copy link
Member Author

OK, this is ready for review, bug report is written.

@schveiguy
Copy link
Member Author

schveiguy commented Mar 31, 2022

An example:

    auto arr = [1,2,3,4,5];
    auto x = cast(long[])arr;

With current library:

An array of size 20 does not align on an array of size 16, so `int` cannot be cast to `long`

With change:

`int[]` of length 5 cannot be cast to `long[]` as its length in bytes (20) is not a multiple of `long.sizeof` (8).

Copy link
Contributor

@adamdruppe adamdruppe left a comment

Choose a reason for hiding this comment

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

i like it, nice small improvement

@MoonlightSentinel
Copy link
Contributor

@schveiguy I've added a unittest for this change.

@schveiguy
Copy link
Member Author

@MoonlightSentinel thanks I didn't know how to make a unittest with this. Obvious in hindsight!

@PetarKirov
Copy link
Member

How about:

`int[]` cannot be cast to `long[]` as its length in bytes (20) is not a multiple of `long.sizeof` (8).

@schveiguy
Copy link
Member Author

@PetarKirov nice! I will update when at home.

@schveiguy
Copy link
Member Author

@PetarKirov I left in the array length, as I still don't want the user to have to do too much math. But should be good now.

@dlang-bot dlang-bot merged commit 2b006e7 into dlang:master Mar 31, 2022
@schveiguy schveiguy deleted the fixarraycast branch April 1, 2022 01:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Enhancement New functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants