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

allow converting static vars to openArray #19049

Merged
merged 1 commit into from
Oct 27, 2021
Merged

allow converting static vars to openArray #19049

merged 1 commit into from
Oct 27, 2021

Conversation

etan-status
Copy link
Contributor

When assigning constant output to a seq, and then passing that static
seq to other functions that take openArray, the compiler may end up
producing errors, as it does not know how to convert static[seq[T]]
to openArray[T]. By ignoring the static wrapper on the type for
the purpose of determining data memory location and length, this gets
resolved cleanly. Unfortunately, it is relatively tricky to come up
with a minimal example, as there are followup problems from the failing
conversion, e.g., this may lead to internal error: inconsistent environment type, instead of the relevant openArrayLoc error message.

(cherry picked from commit 490c422)

When assigning constant output to a seq, and then passing that static
seq to other functions that take `openArray`, the compiler may end up
producing errors, as it does not know how to convert `static[seq[T]]`
to `openArray[T]`. By ignoring the `static` wrapper on the type for
the purpose of determining data memory location and length, this gets
resolved cleanly. Unfortunately, it is relatively tricky to come up
with a minimal example, as there are followup problems from the failing
conversion, e.g., this may lead to `internal error: inconsistent
environment type`, instead of the relevant `openArrayLoc` error message.

(cherry picked from commit 490c422)
@etan-status
Copy link
Contributor Author

The CI error in nimpy from packages OSX_amd64_pkg is new, but is due to upstream changes in nimpy that were not tested in the previous 1.2 commit. The error is not related to this PR.

See https://github.com/yglukhov/nimpy/compare/5621afccfc0903e476a440c953daeb5501d99b00..master
AssertionDefect was introduced but does not work on 1.2.

@etan-status
Copy link
Contributor Author

The CI error in packages Windows_amd64 was already present before on 1.2 and is not introduced by this PR.

@narimiran
Copy link
Member

narimiran commented Oct 25, 2021

We usually don't make PRs to stable branches. PRs are merged into devel, and then backported (by cherry-picking) to the branches of stable versions.

EDIT: I was on outdated devel, and didn't notice this was already in it, and this was a cherry-pick from there. Sorry for the confusion.

@narimiran narimiran closed this Oct 25, 2021
@narimiran narimiran reopened this Oct 25, 2021
@etan-status
Copy link
Contributor Author

Thanks, and yes, these are indeed cherry-picks from the already-accepted devel PR to bring this fix back to the older version. Of particular interest for us would be the version-1-2 flavor here, on which the issue was originally discovered.

@narimiran narimiran merged commit 735e3ee into nim-lang:version-1-2 Oct 27, 2021
@etan-status etan-status deleted the static-openarray-1-2 branch October 27, 2021 09:25
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.

2 participants