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

Tighten validation of hvncat implementation #43940

Merged
merged 3 commits into from
Feb 16, 2022

Conversation

BioTurboNick
Copy link
Contributor

@BioTurboNick BioTurboNick commented Jan 26, 2022

As documented in #43933 , a user may run into an unexpected array if they attempt to concatenate arrays using the multidimensional syntax but don't know the semicolon precedence rules:

[[1 1]; 2 ;; 3 ; [3 4]]
[[1 ;;; 1]; 2 ;;; 3 ; [3 ;;; 4]]

Naively, one might think these expressions would work, assuming ; had lowest precedence. The _typed_hvncat_dims implementation needed to do more to check the dimensions of the arguments.

Fixes #43933

Should be backported to 1.7, yes?

@BioTurboNick

This comment has been minimized.

@oscardssmith oscardssmith added backport 1.7 bugfix This change fixes an existing bug labels Jan 26, 2022
@BioTurboNick
Copy link
Contributor Author

Looks like it works! Slightly slower (~3%), no new allocations. Would be nice not to have to iterate over the arguments twice, but it's a small cost.

@BioTurboNick BioTurboNick marked this pull request as ready for review January 26, 2022 21:17
@KristofferC KristofferC mentioned this pull request Feb 15, 2022
40 tasks
@KristofferC KristofferC added the backport 1.8 Change should be backported to release-1.8 label Feb 16, 2022
@KristofferC KristofferC merged commit 76dc379 into JuliaLang:master Feb 16, 2022
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request Feb 17, 2022
@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label Feb 18, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
KristofferC pushed a commit that referenced this pull request Feb 23, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Weird outputs of concatenated arrays
3 participants