Skip to content

Add std.typecons.HeadConst #2881

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

Closed
wants to merge 1 commit into from
Closed

Conversation

JakobOvrum
Copy link
Member

A simple type constructor that replicates the functionality of the now-defunct final variable storage class, allowing for easy creation of "read-only" variables and fields without going the full mile with transitive immutability. Using Final is quite a bit shorter than using a private variable with a public getter, and it's easier to get right since using a property correctly requires understanding of inout and the various function attributes. I guess it's also a nice thing to point to when queried about by Java programmers, and maybe even useful for porting Java code to D.

Please destroy.

this(Args...)(auto ref Args args)
if(__traits(compiles, T(args)))
{
this.data = data;
Copy link
Contributor

Choose a reason for hiding this comment

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

this.data = T(args); ?

Copy link
Member Author

Choose a reason for hiding this comment

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

That would do exactly the same thing.

edit:

Added a test for construction with multiple arguments.

Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I see, your code is the same as

this.data = this.data;

which is practically a no-op. Your unittest does not check if any field actually gets assigned. Of course it compiles, but it does not do anything meaningful.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, I intended to do this.data = args;. Thanks!

@JakobOvrum JakobOvrum force-pushed the final branch 2 times, most recently from fe0821e to d309a1e Compare January 20, 2015 10:14
@schuetzm
Copy link
Contributor

How about calling it HeadConst? Final will only be understood by people coming from D1, and can also be confused with final classes.

@JakobOvrum
Copy link
Member Author

D1 programmers, but also Java programmers. Final is also the name used to describe the same type in TDPL. HeadConst seems a little clumsy to me, but I'd love to hear what other people think the name should be.

@MetaLang
Copy link
Member

If someone knows what a final class is, then they've probably used Java, and so they probably know what it means to mark a variable as final.

@mihails-strasuns
Copy link

I also find Final confusing. My first thought reading the PR name was "how it is related to final methods?"
Something like HeadConst or ReadOnly would be more obvious.

@PetarKirov
Copy link
Member

+1 for naming it HeadConst. I find it far more descriptive than Final, which can cause confusion. (ReadOnly doesn't say anything about the transitivity.)

@JakobOvrum
Copy link
Member Author

Renamed it to HeadConst.

@JakobOvrum JakobOvrum changed the title Add std.typecons.Final Add std.typecons.HeadConst Jan 23, 2015
@ghost
Copy link

ghost commented Jan 25, 2015

LGTM!

@andralex can we get an approval for this?

{
int i;

pure nothrow @safe:
Copy link
Collaborator

Choose a reason for hiding this comment

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

tabs2spaces please.

@monarchdodra
Copy link
Collaborator

No headConst helper-builder?


public:
/// $(D HeadConst) subtypes $(D T) as an rvalue.
inout(T) HeadConst_get() inout pure nothrow @nogc @safe
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this actually a public function we want users to be able to use? I realize it probably needs to be public for the bellow alias to work, but let's not document it that way.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not pure nothrow (etc...), as it depends on the attributes applied to T's postblit.

Speaking of which, it might be worth having a const-by-ref overload.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is this actually a public function we want users to be able to use? I realize it probably needs to be public for the bellow alias to work, but let's not document it that way.

Yeah, it's public for AliasThis to work. I chose to document it because it's currently not possible to document an AliasThis declaration with DDoc, but it could simply be mentioned in the type description instead.

This is not pure nothrow (etc...), as it depends on the attributes applied to T's postblit.

You're right, I was hoping the postblit operator would be called in the environment of the callsite :( Removing the explicit attributes and relying on inference seems to work well.

Speaking of which, it might be worth having a const-by-ref overload.

I tried this, but the const-by-ref overload is never chosen when the this-reference is const, and when I make it inout like the other overload, I get "matches both" errors.

@monarchdodra
Copy link
Collaborator

I like the idea, but I am less than thrilled about the following behavior:

import std.stdio;

struct S
{
    int i;
    void doit() {
            ++i;        
    }
}

void main()
{
    HeadConst!S s;
    s.i = 5; //No-op
    writeln(s.i);
    s.doit(); //No-op
    writeln(s.i);
}

I realize S is a value type, but still.

At the very least, I argue that for non-explicit reference types (classes, pointers, interfaces), than alias this should simply resolve to a const variable. Accessing a mutable copy should be done via an explicit "get()" or "copy()" or something.

Having alias this do anything more than return a ref to a field has always led to problems at one point or another...

@JakobOvrum
Copy link
Member Author

I like the idea, but I am less than thrilled about the following behavior:

Ouch, didn't notice that one. Maybe some opDispatch magic can fix it, but there could be other problems... I'll try some things.

At the very least, I argue that for non-explicit reference types (classes, pointers, interfaces), than alias this should simply resolve to a const variable. Accessing a mutable copy should be done via an explicit "get()" or "copy()" or something.

I think this type is much less appealing if it's not a drop-in replacement for T.

Having alias this do anything more than return a ref to a field has always led to problems at one point or another...

Although it would make the implementation much more involved, we could always go the Proxy approach. But I'll see if I can make something work with AliasThis.

@JakobOvrum
Copy link
Member Author

I'm going to close this for now and prioritise other things. If someone wants to spearhead this they are more than welcome to pick up where I left it. I think a correct implementation needs to use operator overloading (notably opDispatch) like Proxy does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants