Skip to content
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

Introduce package level lvalueInit #2172

Closed
wants to merge 3 commits into from
Closed

Introduce package level lvalueInit #2172

wants to merge 3 commits into from

Conversation

monarchdodra
Copy link
Collaborator

This is a library solution for of my original "WONTFIX" request:
"Make const(T).init and immutable(T).init lvalues"
https://issues.dlang.org/show_bug.cgi?id=11307

It is currently used in 3 places, but I have plans to add it to 2 other places too (initializeAll and a moveOver required for emplacing rvalues with disabled postblit).

It also fixes a bug where move could not be used with elaborate types that have a disabled default initialization.

Overall, it's just a little convenience that avoids bloat.

@monarchdodra
Copy link
Collaborator Author

For Pete's sake... why isn't this linking... I hate the Darwin release :/ Anybody have any ideas?

I'll try to reduce and reproduce tomorrow :(

static if (is(T == immutable(T)))
static lvalueInit = T.init;
else
alias lvalueInit = .lvalueInit!(immutable(T));
Copy link

Choose a reason for hiding this comment

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

alias? Shouldn't this be static or enum?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The "instance proper" is instanciated inside the immutable(T) template. If T is not immutable, then the template aliases to the immutable version.

I'll try removing the code, just as a test in regards to the link problem, but it should be valid (and necessary to guarantee consistency).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah... that didn't help :/

Copy link

Choose a reason for hiding this comment

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

Shame.. It smells like a compiler bug. Maybe ping Kenji about it.

@monarchdodra
Copy link
Collaborator Author

Shame.. It smells like a compiler bug.

Reduced and filed: https://issues.dlang.org/show_bug.cgi?id=12754

Maybe ping Kenji about it.

I'll try pinging both @WalterBright and @9rnsr , though it seems like a Darwin-linker specific issue.

@quickfur
Copy link
Member

ping
Any updates for the linker issue?

@monarchdodra
Copy link
Collaborator Author

ping
Any updates for the linker issue?

Not really, and there's nothing I can really do on my end, except ping @WalterBright and/or @9rnsr again.

@ghost ghost added the blocked label Aug 4, 2014
@monarchdodra
Copy link
Collaborator Author

This is still blocked, but I think I found a T.init loophole. @AndrejMitrovic :
http://forum.dlang.org/post/tajufmbwezabjohvzwkp@forum.dlang.org

@andralex
Copy link
Member

Is this worth it? What's the benefit?

@DmitryOlshansky
Copy link
Member

@andralex to ease template constraints.
To test that a function takes l-value with type that may have @disabled postblit one have to ((in general) create some empty function returning T as ref.

@monarchdodra
Copy link
Collaborator Author

It also avoids either of:

  • Proliferation of T.init in static space.
  • Declaration of useless temps (and their potential destructor side effects).

The benefits are not huge, but there's no real downside either (and its internal), so worth it, yes, IMO. It just needs to pass Darwin though...

@andralex
Copy link
Member

OK I think this comes up often enough to deserve a place in the stdlib. But let's call it lvalueof!T and never define it.

// static lvalueInit = T.init;
//else
// alias lvalueInit = .lvalueInit!(immutable(T));
static lvalueInit = immutable(T).init;
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 it should be an undefined function, a la:

ref T lvalueof(T)();

True it can't be applied everywhere but it's a better template constraint that's impossible to misuse (e.g. write to).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If you want something that is only for template constraints, we already have lvalueof in std.traits (http://dlang.org/phobos/std_traits.html#lvalueOf). It has the exact signature you propose (give or take an "inout") workaround.

However, I see this as different concepts, since the idea here is to specifically be able to grab a value. You say "impossible to misuse " but:

  • lvalueInit is immutable, so can't be written too anyways.
  • Using a function (I also objected to this with lvalueof) means operator & has a surprising effect, in that it doesn't give you the address of your lvalue, which, IMO, kind of breaks the intent of the damn thing.

@DmitryOlshansky
Copy link
Member

@monarchdodra is it still blocked? Are you around to try to continue this?

@monarchdodra
Copy link
Collaborator Author

@monarchdodra is it still blocked? Are you around to try to continue this?

Honestly, I'm not sure it was ever blocked. Just no one thought it was important enough to merge. I was planning to use it in future code, but I don't think that's going to happen anymore. Let's just close it.

@MartinNowak
Copy link
Member

Can we at least get that as an internal helper?
Would save on object space b/c different functions could use the same initializer.
Ideally the memory would be identical to typeid(T).init(), but I'm not sure dmd is smart enough for that.

@MartinNowak
Copy link
Member

Ideally the memory would be identical to T.init(), but I'm not sure dmd is smart enough for that.

In that case it might be best to keep on using the typeid(T).init() idiom.

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.

5 participants