Skip to content

Conversation

timotheecour
Copy link
Member

this allows safer code than using cast directly in user code, and its lack in stdlib is a common complaint, having it in stdlib instead of as a separate nimble package makes sense for something that common; it also enables using it in compiler/nim sources

@Varriount
Copy link
Contributor

How often are these kinds of facilities even used though? If one needs to increment/decrement a pointer, 95% of the time the type is better represented as a pointer to an unchecked array

Comment on lines +22 to +26
template `[]`*[T](p: ptr T, off: int): T =
(p + off)[]

template `[]=`*[T](p: ptr T, off: int, val: T) =
(p + off)[] = val
Copy link
Member

Choose a reason for hiding this comment

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

These are far too dangerous to have this innocent, common syntax.

@Araq
Copy link
Member

Araq commented Oct 5, 2020

In my code I prefer to have operators like +! and -! to emphasize a little bit that something unusual and unsafe is going on.

@juancarlospaco
Copy link
Collaborator

Instead I would prefer to see implicitDeref moved out of --experimental now that we have pools module that uses only ptr.

@timotheecour
Copy link
Member Author

timotheecour commented Oct 5, 2020

Instead I would prefer to see implicitDeref moved out of --experimental now that we have pools module that uses only ptr.

I don't see how implicitDeref relates to or circumvents this PR, std/pointers is among other things useful for:

In my code I prefer to have operators like +! and -! to emphasize a little bit that something unusual and unsafe is going on.

using non-standard operator names isn't good; + has one obvious meaning for ptr T just like it does in C, C++, D, swift etc (all support pointer arithmetic as in this PR); unlike for eg for concatenation where nim rightfully departs from python and uses & to avoid ambiguities. The leibniz argument applies.

Most code would not deal with pointer indexing and code that does will have explicit import std/pointers anyways [1]

regarding safety:

A misused cast causes no compilation errors.

using operators from std/pointers is arguably safer than inlining their code directly as is done in many places; writing directly cast[ptr T](cast[ByteAddress](p) +% off * sizeof(T)) is both un-readable (obscuring code intent and increasing likelyhood of bugs) and error prone since user may get T wrong for example.

[1] and import at local scope (as I've implemented in a local fork) makes it even clearer

@dom96
Copy link
Contributor

dom96 commented Oct 5, 2020

How about implementing this so that it's only possible within a unsafe code block? That would be a pretty nice feature and would make it clear that the code is dangerous.

@timotheecour
Copy link
Member Author

timotheecour commented Oct 5, 2020

How about implementing this so that it's only possible within a unsafe code block? That would be a pretty nice feature and would make it clear that the code is dangerous.

good idea, maybe. one caveat is that cast doesn't require it (since {.unsafe.} doesn't exist yet), yet is even less safe. it could look like this:

import std/pointers
proc fn=
  let a = getPtr()
  {.unsafe.}:
    let b = a+1
    echo b[2]
  echo b[] # b is in scope

(it'd be analog to {.gcsafe.}:)

@timotheecour
Copy link
Member Author

see alternative approach here: #15500

@Varriount
Copy link
Contributor

In my code I prefer to have operators like +! and -! to emphasize a little bit that something unusual and unsafe is going on.

If we have to have a module dedicated to pointer arithmetic (and I still don't feel that there's much call for one) I would much prefer this over regular arithmetic operators.

@dom96
Copy link
Contributor

dom96 commented Oct 6, 2020

one caveat is that cast doesn't require it (since {.unsafe.} doesn't exist yet), yet is even less safe.

That's not a caveat, as someone that uses Nim you should learn to grep for cast/ptr/addr as a signal of unsafety.

There is an argument to be made that Nim should have only allowed it under an unsafe block like Rust, but we've had that discussion before... and now the ship has sailed.

@Araq
Copy link
Member

Araq commented Oct 6, 2020

There is an argument to be made that Nim should have only allowed it under an unsafe block like Rust, but we've had that discussion before... and now the ship has sailed.

There is nothing to regret here, Rust's way doesn't work as well as Nim's and both don't really work all that well when you use extensive interfacing to C(++).

@alaviss
Copy link
Collaborator

alaviss commented Oct 13, 2020

Given that the pointers module migrated to fusion I believe this can be closed now?

@timotheecour
Copy link
Member Author

it's still pending nim-lang/fusion#21

@Araq
Copy link
Member

Araq commented Sep 19, 2021

Nevertheless it should be in fusion. Or in your own Nimble package.

@Araq Araq closed this Sep 19, 2021
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