Skip to content

Conversation

@WalterBright
Copy link
Member

A start to the eventual improvement where unique pointers can be implicitly converted to immutable and shared.

@MartinNowak
Copy link
Member

What's a mutable unique pointer?
Also a test case would be useful.

@braddr
Copy link
Member

braddr commented Feb 27, 2013

Design docs and discussion before implementation please. You'd expect no less from a community contribution.

@WalterBright
Copy link
Member Author

A unique pointer is one for which no copies exist, so it can be safely converted to immutable or shared. Operator new, for example, creates a unique pointer (absent what a constructor might do).

This was discussed on the D n.g. a couple months ago.

It's not possible to do a test case for this particular one as the other stuff isn't there.

@WalterBright
Copy link
Member Author

@braddr
Copy link
Member

braddr commented Feb 27, 2013

I skimmed through the threads and didn't see a clear proposal. I saw a lot of talking and a lot of thoughts. You need to do what we're strongly encouraging everyone else to do, formalize the plan. Let people poke at it, enhance it. If you want to start a branch and play with an implementation then great. Merging pieces into the master branch absent either or both of those is premature and exactly the behavior you get grief from the community about doing.

@yebblies
Copy link
Contributor

You should be able to add test cases for this. How do you know it works if you haven't tested?

This test case should work (it doesn't)

int* pureMaker() pure
{
    return [1,2,3,4];
}

void main()
{
    immutable x = pureMaker() + 3;
}

You're checking that typeb->castMod(0) converts to tb->castMod(0), in this case it yeilds int* and immutable(int)*, which of course doesn't work. Maybe you meant t->invariantOf() which is IIRC what the existing 'unique' conversion code does.

And then of this is unnecessary, because you've special cased it to only allow offsetting pointers, where the only thing you need to know is that the original pointer converts to the final type. If it does, no amount of offsetting will break that. This should already be handled, because you're calling implicitConvTo on it. (NewExp, CatExp, ArrayLiteralExp, pure CallExp etc should eventually all be handled in here)

implicitConvToCommon is a misleading name, it should reflect what it actually does. implicitConvToUnique? Or if this is only meant to handle pointer offsetting, it should be named to show that. pointerOffsetUnique? Otherwise the special casing should be moved into the correct implicitConvTo functions.

@WalterBright
Copy link
Member Author

The test case doesn't work because there's no code to determine that pureMaker returns a unique pointer.

And then of this is unnecessary, because you've special cased it to only allow offsetting pointers, where the only thing you need to know is that the original pointer converts to the final type. If it does, no amount of offsetting will break that. This should already be handled, because you're calling implicitConvTo on it.

This is incorrect. The expression must be looked at to see if it generates a unique pointer or not. Looking at the type is insufficient.

@WalterBright
Copy link
Member Author

Yes, Brad, I'll do a dip on it.

@yebblies
Copy link
Contributor

The test case doesn't work because there's no code to determine that pureMaker returns a unique pointer.

Actually, there is, and it's in CallExp::implicitConvTo.

This is incorrect. The expression must be looked at to see if it generates a unique pointer or not. Looking at the type is insufficient.

Please read my message again this is not what I said.

There is already some support for unique expressions in the compiler (from pull #218), and with a couple of tweaks to your patch I made the testcase I posted work.

@WalterBright
Copy link
Member Author

You're right, yebblies, I'd missed that. What are the tweaks you made?

@9rnsr
Copy link
Contributor

9rnsr commented Feb 28, 2013

@yebblies Your example must not work, because pureMaker is not annotated with pure. So pureMaker can return an int pointer of global variable, and immutable x = pureMaker() + 3; will break immutable guarantee.

@WalterBright
Copy link
Member Author

BTW, I think the pull 218 is incorrect. For example,

pure T* foo(T* p) { return p; }

is not implicitly convertible to immutable.

@WalterBright
Copy link
Member Author

Other implicit conversions that should be possible under varying circumstances:

  1. to mutable
  2. to immutable
  3. to shared
  4. to not shared

The cases are rather complicated and not as simple as is/isnot a unique pointer.

I'm unsure what to do about inout.

@yebblies
Copy link
Contributor

@9rnsr Yes, I meant to put pure there.

@WalterBright

What are the tweaks you made?

I completely removed the castMod check. Changing it to use invariantOf also works. This is ok for pointer offsetting because we are not changing the type, just adjusting the value, and the conversion is checked later when calling implicitConvTo.

BTW, I think the pull 218 is incorrect. For example,
pure T* foo(T* p) { return p; }

Pull 218 is correct. The conversion will only be allowed if foo is strongly pure, in which cast T has to be immutable.

@WalterBright
Copy link
Member Author

yebblies, please show your exact patch.

I tried to generalize my code to more than just casting to immutable.

@9rnsr
Copy link
Contributor

9rnsr commented Feb 28, 2013

I can agree that #218 is correct, and recently I have improved the concept in #1519 (not only PUREstrong return, and also PUREconst and PUREweak return may be convertible to immutable implicitly). I call the traits 'isolated return', but based on this pull summary, we can call it "unique return".

And then, I also basically agree with @WalterBright that the returned object from new operation is also unique. As far as I know, that is recently implemented by him. This pull seems to me to extend the idea, and may allow immutable T* p = (new T() + n);

But @WalterBright , please implement it carefully. If T has some mutable indirections, new T(...) + n must not be convertible to immutable implicitly. Does this change take care it? To visualize it, you should add test cases.

@yebblies
Copy link
Contributor

@WalterBright

yebblies, please show your exact patch.

yebblies@2777813

I tried to generalize my code to more than just casting to immutable.

If t converts to t->invariantOf() then t is either immutable or unique. This means you can safely add the shared modifier, giving shared(T) if T was mutable.

Some test cases would help me understand what casts you plan to allow.

@WalterBright
Copy link
Member Author

Testing by converting to immutable is not sufficient. For example, given:

struct S { immutable(int)* p1; shared(int)* p2; }

a pointer to S can be converted to shared, but not to immutable.

All the different cases of this hurts my brain :-)

@WalterBright
Copy link
Member Author

@9rnsr, you are correct in that if operator new is constructing an object that has a constructor, the constructor must be regarded like a function call in order to determine if the resulting new'd pointer is unique. The changes I made earlier sidestepped the issue by rejecting new expressions that call constructors.

@WalterBright
Copy link
Member Author

I made a start on a DIP: http://wiki.dlang.org/DIP29

It's very incomplete.

@yebblies
Copy link
Contributor

yebblies commented Mar 1, 2013

@WalterBright

Testing by converting to immutable is not sufficient. For example, given:
struct S { immutable(int)* p1; shared(int)* p2; }
a pointer to S can be converted to shared, but not to immutable.

I think it can still be converted to immutable if the expression is unique.

All the different cases of this hurts my brain :-)

Mine too.

@9rnsr
Copy link
Contributor

9rnsr commented Apr 12, 2013

This test case should work with the change, but doesn't.

template TypeTuple(T...) { alias T TypeTuple; }
void main()
{
    static pure T* makeUniqueHead(T)(size_t dim) { return (new T[](dim)).ptr; }
    static pure T* makeUniqueLast(T)(size_t dim) { return (new T[](dim)).ptr + dim - 1; }
    static struct S1 { int val; }
    static struct S2 { int[] arr; }

    foreach (T; TypeTuple!(int, S1, S2))
    {
        immutable T* p0 = makeUniqueHead!T(2);      // OK
        immutable T* p1 = makeUniqueHead!T(2) + 1;  // should be OK, but NG

        immutable T* q1 = makeUniqueLast!T(2);      // OK
        immutable T* q0 = makeUniqueLast!T(2) - 1;  // should be OK, but NG
    }
}

@WalterBright
Copy link
Member Author

Replaced by #3319

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.

5 participants