Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

Add mechanism for pointers that prevents dereferencing#1592

Closed
schveiguy wants to merge 4 commits intodlang:masterfrom
schveiguy:noderefptr
Closed

Add mechanism for pointers that prevents dereferencing#1592
schveiguy wants to merge 4 commits intodlang:masterfrom
schveiguy:noderefptr

Conversation

@schveiguy
Copy link
Member

Inspired by @WalterBright's changes for @safe access to arr.ptr, this attempts to make a pseudo-pointer type that does everything a pointer can do, except dereference, and create a pointer at a relative location.

The goal is to have something that acts like a pointer (i.e. can compare, subtraction yields signed result), but is completely @safe. It also provides a mechanism to get back to the pointer, but only in @system code. There are many times you don't care at all about what a pointer points at, but just where it is in memory. Or you want to enforce this rule for @safe code (which does allow dereferencing normal pointers), or even @system code.

See #1590 and dlang/phobos#4427

Note: I put this in core.internal for now, but I'm hoping we can move it to object.d as a general mechanism. All names up for debate.

Some issues with this code:

  • Does not implicitly cast to void * equivalent PtrVal, you have to use toVoid conversion function (no language support for this)
  • I tried to avoid having e.g. PtrVal!(const(int)) but instead opted for const(PtrVal!int). This makes things much more sane, but there is no tail-const support because of this.
  • I had to add some Phobos traits code to be able to copy modifiers. I did not copy the appropriate unit tests, but I think that's standard practice.

like pointers for pointer comparison and math. All usable with @safe
code.

/*
* This allows safe use of a pointer without dereferencing. Useful when you
* want to declare that all you care about it the pointer value itself. Keeps
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: s/it/is/

@schveiguy
Copy link
Member Author

Stupid windows makefiles...

* Use this if you are in system code and wish to get back to unsafe
* pointer-land.
*/
inout(T) *ptr() inout @system
Copy link
Member

Choose a reason for hiding this comment

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

I think this shouldn't be inout since your returning a copy of the pointer and if the user wants a const pointer, then PtrVal!(const T*) should be used.

Copy link
Member Author

@schveiguy schveiguy Jun 14, 2016

Choose a reason for hiding this comment

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

You misunderstand why this is inout. If you have a const(PtrVal!(...)), then you can't call ptr on it unless the function is marked const or inout. I'd need several copies of the same function. This is the whole point of inout.

@PetarKirov
Copy link
Member

@schveiguy maybe I didn't explain well, but I expected to see something like this:

struct PtrVal(T) if (isPointer!T)
{
    private T _ptr;
    T ptr const @system { return _ptr; }
    // the rest is the same
}

PtrVal!T ptrval(T)(T ptr) if (isPointer!T)
{
    return PtrVal!T(ptr);
}

Why did you decide to instead templatize on the pointer target type and not the pointer type itself?

@PetarKirov
Copy link
Member

In other words, I think it's more straightforward for the users (those that don't use arr.ptrval) that if they want to put a const pointer, they will do PtrVal!(const T*) and not const(PtrVal!T).
For example, instead doing PtrVal!(PointerTarget!(typeof(val))), with my approach the users would do PtrVal!(typeof(val)) in generic code, which I think is more straightforward. I know you can do typeof(*val) but that's not the point, because you can't easily do this with e.g. std.meta.staticMap.

@schveiguy
Copy link
Member Author

schveiguy commented Jun 15, 2016

Isn't this pretty easy?

alias PtrValFor(T) = PtrVal!(typeof(*T.init));

In any case, I look at it much differently. When I want to say "pointer to int", I write int *. I specify the base type, and then a * to signify it is a pointer. It follows naturally to me that PtrVal!T means "pointer value to T". Your way is like saying Array!(int[]). Is it an array of int, or an array of int arrays?

The code is much more straightforward, there are no template constraints because you can't write an incorrect PtrVal.

Regarding the const outside vs inside -- the language has poor support for this:

  1. implicit conversions don't exist between PtrVal!T and PtrVal!(const(T)), but they do for const(PtrVal!T).
  2. inout will not work for that at all (an incorrect limitation, but one nonetheless).
  3. e.g. opBinary!"-" would have to be templated on the PtrVal parameter, and I'd have to add constraints to make sure it's correct.

My way is much simpler. Yes, it makes for issues when you want to rebind const pointers. But I don't see this being a tremendously difficult issue to deal with. If we ever get proper tail-modifier support, this problem goes away. It may be that if we add this mechanism to the language (and not just in core/internal), then we can revisit fixing some corner cases.

@PetarKirov
Copy link
Member

PetarKirov commented Jun 16, 2016

Yes, I understand that you're comming from the type constructor perspective (PointerOf!T -> T*), but I expected that PtrVal is value type just like uint_ptr_t and for me those type qualifier gymnastics like typeof(const(PtrVal!int).init.ptr) == const(int)*, are quite awkward. IOW, I didn't expect const transitivity.

About the specific points you raised:

  1. My version doesn't support implicit conversions, and that's on purpose.
  2. My version doesn't use inout anywhere (only const).
  3. Since no conversions are allowed, there's no need to make opBinary templated on its parameter.

In the end, it all comes down to the question: Is this value type, or pointer type in a safe disguise? Currently it looks like you have more of a SafePtr, than PtrVal.

* Cast to void pointer type. TODO: see if there is a way to make this
* implicit.
*/
auto toVoid() inout @trusted
Copy link
Member

Choose a reason for hiding this comment

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

BTW, you can make this a template (with no params) to make the import optional.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, not a bad idea.

@schveiguy
Copy link
Member Author

I expected that PtrVal is value type just like uint_ptr_t

One thing about this is that you can get the pointer back. It can't be a value type (and actually, in order to be properly scanned by the GC, it must have a pointer internally).

for me those type qualifier gymnastics like typeof(const(PtrVal!int).init.ptr) == const(int)*, are quite awkward

As opposed to: typeof(PtrVal!(const(int)).init.ptr) == const(int)* ? I guess I don't see much difference, perhaps you can elaborate. Note that this is still possible, it's just not preferred.

My version doesn't use inout anywhere (only const).

I'm not sure why this is a benefit.

Since no conversions are allowed, there's no need to make opBinary templated on its parameter.

Shouldn't this work? With your proposed usage, I'd think you'd have to template opBinary.

string x;
char[] y;

auto d = x.ptrval - y.ptrval;

This is a pretty small type, perhaps the easiest thing to do is for you to post what you think the code should be, and we can discuss the differences. I'm certainly willing to listen to a different point of view, but I want to know exactly what you are talking about.

Keep in mind that the main purpose of this type is to allow a drop-in replacement for arr.ptr for @safe code in all uses except dereferencing.

@dlang-bot dlang-bot added Needs Rebase needs a `git rebase` performed stalled Needs Work labels Jan 1, 2018
@timotheecour
Copy link
Contributor

somewhat related: dlang/phobos#6231

@schveiguy
Copy link
Member Author

I had forgotten about this. I'm going to just close it, as I don't think it will be merged.

@schveiguy schveiguy closed this Feb 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Needs Rebase needs a `git rebase` performed Needs Work stalled

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants