Skip to content

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Oct 6, 2020

alternative to #15490

adds toUncheckedArray

# before
let px = cast[ptr UncheckedArray[type(x)]](x)
# after
let px = x.toUncheckedArray

it provides a safer and more readable construct than the explicit cast (which is error prone since the cast will happily accept any type without giving any compiler error, eg in case of let px = cast[ptr UncheckedArray[int]](pointerToInt8)

@juancarlospaco
Copy link
Collaborator

juancarlospaco commented Oct 6, 2020

Documentation needs a warning that is unsafe,
and I think you can not guarantee is not "error prone" so maybe remove that comment.

@Araq
Copy link
Member

Araq commented Oct 6, 2020

This is going in the right direction but addr(a[i]).toUncheckedArray is messier than e.g. a.toUncheckedArray(i). Don't take the address of a single element and then cast it to a fullblown array. It was an array, it is turned into a pointer to an UncheckedArray.

@timotheecour
Copy link
Member Author

timotheecour commented Oct 6, 2020

addr(a[i]).toUncheckedArray is messier than e.g. a.toUncheckedArray(i). Don't take the address of a single element and then cast it to a fullblown array. It was an array, it is turned into a pointer to an UncheckedArray.

your assumption is too restrictive, toUncheckedArray is meant to be usable for dealing with any ptr[T] wherever they come from (eg FFI or addr of something), where you can't assume it's an array[T] or seq[T] etc; it's the most general case

@timotheecour
Copy link
Member Author

PTAL

@Araq
Copy link
Member

Araq commented Oct 8, 2020

This is a new std module that nobody uses, with no RFC process to back it up, without use cases to justify it and not useful for the low level code I typically write (I need "pointer +- offset"). I don't understand why we need more obviously not battle-tested standard modules. Why not add it to "fusion" instead?

@disruptek
Copy link
Contributor

How is that any different from fusion?

Copy link
Contributor

@dom96 dom96 left a comment

Choose a reason for hiding this comment

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

What Araq said. Additions to the stdlib should be well thought through, discussed and hopefully tested ahead of time as a Nimble package

@timotheecour
Copy link
Member Author

timotheecour commented Oct 8, 2020

@Araq

Why not add it to "fusion" instead?

=> nim-lang/fusion#20
(fusion is a better fit than a separate nimble package)

I'll close this PR if the fusion one gets merged

@timotheecour
Copy link
Member Author

=> nim-lang/fusion#20

@timotheecour timotheecour deleted the pr_toUncheckedArray branch October 9, 2020 08:15
@timotheecour timotheecour changed the title new pointers.toUncheckedArray [superseded] new pointers.toUncheckedArray Oct 9, 2020
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.

6 participants