Skip to content

Comments

fix Issue 14432 - move construction for RefCounted#3171

Merged
JakobOvrum merged 3 commits intodlang:masterfrom
MartinNowak:refCounted
Apr 20, 2015
Merged

fix Issue 14432 - move construction for RefCounted#3171
JakobOvrum merged 3 commits intodlang:masterfrom
MartinNowak:refCounted

Conversation

@MartinNowak
Copy link
Member

  • add RefCounted!T.this(T) which takes an RValue or a copy
    and use move to initialized the refcounted store
  • add refCounted to infer T from the argument and disable
    RefCounted's autoInit, also move initializes the store

std/typecons.d Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

You can't use enforce for malloc, use core.exception.onOutOfMemoryError.

See also #3031.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

- add RefCounted!T.this(T) which takes an RValue or a copy
  and use move to initialized the refcounted store

- add refCounted to infer T from the argument and disable
  RefCounted's autoInit, also move initializes the store
std/typecons.d Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Why put this in TLS instead of the stack? Could also use typeid(T).init() like destroy does.

Copy link
Member Author

Choose a reason for hiding this comment

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

Mmh, I just copied the move code. Updated.

@MartinNowak
Copy link
Member Author

Also changed move to use typeid(T).init() instead of an "empty" TLS variable.
This stuff would be way move efficient, if we had destructive move.

@MartinNowak
Copy link
Member Author

Once #3139 is ready, we should specifically handle promoting a Unique!T to a RefCounted!T.

@JakobOvrum
Copy link
Contributor

LGTM

@MartinNowak
Copy link
Member Author

Well then go ahead merge it ;).

@JakobOvrum
Copy link
Contributor

Ideally a third person would look over it, but it's already been 10 days so...

@JakobOvrum
Copy link
Contributor

Auto-merge toggled on

JakobOvrum added a commit that referenced this pull request Apr 20, 2015
fix Issue 14432 - move construction for RefCounted
@JakobOvrum JakobOvrum merged commit f7498ad into dlang:master Apr 20, 2015
@MartinNowak MartinNowak deleted the refCounted branch April 20, 2015 14:38
@MartinNowak
Copy link
Member Author

Ideally a third person would look over it

Ideally we'd handle PR within a few days.
I made a best-effort implementation, you did a best-effort review, there is nothing wrong with making mistakes.

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.

2 participants