AlignedStorage and InstanceStorage types #5426

radcapricorn · 2017-05-24T19:07:25Z

These are two untyped storage types that can be used in Phobos
and in user code when there's a need for specific aligned space.
E.g. we could use this as static storage to emplace() objects
instead of allocating them with GC, where functions need to be made @nogc.

radcapricorn · 2017-05-24T23:49:15Z

Not sure why tests would fail on win32. Can anyone offer insight?

std/typecons.d

radcapricorn · 2017-05-25T04:14:48Z

Gah... So there is no way of telling at compile time whether invariants are enabled or not?

MetaLang · 2017-08-06T04:41:52Z

@andralex I tagged you because I can't remember if it's all Phobos symbols, or only std.algorithm/std.range that you want to be notified of.

andralex

What is the motivating case for this? How is it different from simply placing align(x) on the needed member?

std/typecons.d

radcapricorn · 2018-11-25T08:32:44Z

@andralex This need not necessarily be a member, it could be a local, a global, an allocated storage...
I'll be fixing and rebasing this shortly.

radcapricorn · 2018-11-25T08:33:59Z

@thewilsonator unstall please, I'm resuming this :)

std/typecons.d

thewilsonator · 2018-11-25T10:21:04Z

Please rebase it then ;)

dlang-bot · 2018-11-28T11:37:25Z

Thanks for your pull request and interest in making D better, @radcapricorn! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
I have provided a detailed rationale explaining my changes
New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + phobos#5426"

thewilsonator · 2018-11-28T11:41:16Z

std/typecons.d(9074:8)[warn]: Public declaration 'AlignedStorage' has no documented example.
std/typecons.d(9108:6)[warn]: Trusting a whole scope is a bad idea, `@trusted` should only be attached to the functions individually
std/typecons.d(9119:1)[warn]: A unittest should be annotated with at least @safe or @system
std/typecons.d(9128:1)[warn]: A unittest should be annotated with at least @safe or @system
std/typecons.d(9152:1)[warn]: A unittest should be annotated with at least @safe or @system
std/typecons.d(9171:10)[warn]: Public declaration 'InstanceStorage' has no documented example.
std/typecons.d(9171:33)[warn]: If constraints should have the same indentation as the function
std/typecons.d(9184:1)[warn]: A unittest should be annotated with at least @safe or @system
std/typecons.d(9194:1)[warn]: A unittest should be annotated with at least @safe or @system
std/typecons.d(9210:1)[warn]: A unittest should be annotated with at least @safe or @system
std/typecons.d(9232:1)[warn]: A unittest should be annotated with at least @safe or @system

These are two untyped storage types that can be used in Phobos and in user code when there's a need for specific aligned space. E.g. we could use this as static storage to emplace() objects instead of allocating them with GC, where functions need to be made @nogc. Added `cent` check, improved coverage. unittest versioning; typo Removed invariant test cent fix rebase, remove format Example tests, style fixes

andralex

I think some serious convincing is needed on why this belongs in the standard library.

andralex · 2018-11-28T21:46:45Z

std/typecons.d

+at compile time, an assert will be issued at runtime when a misaligned
+AlignedStorage is used.
+*/
+struct AlignedStorage(size_t Size, size_t Alignment = platformAlignment)


This somewhat sophisticated API is ill-advised. We're looking at an "abstraction that doesn't abstract": a data member that is nominally private, yet trivially made public by means of accessors. There's no need for APIs for length, slicing etc because all they do is morally equivalent with:

struct AlignedStorage(size_t Size, size_t Alignment = platformAlignment) { align(Alignment) void[Size] payload; }

Everything needed is readily accessible by means of .payload. This also emphasizes the question of why we need to put in the standard library what's essentially a three-line trick.

...with the exception of an invariant that safeguards against misaligned moves :)

andralex · 2018-11-28T21:48:13Z

std/typecons.d

+For classes, the size and alignment of storage are the size and alignment
+of class instance representation, not those of class reference.
+*/
+template InstanceStorage(T)


nicely done

andralex · 2018-11-28T21:48:33Z

std/typecons.d

+    static assert(InstanceStorage!Arbitrary.alignof == Arbitrary.alignof);
+}
+
+private size_t platformAlignment() @safe @nogc nothrow pure


I think we have something like this in std.experimental.allocator.

Yes. What would you suggest? Make this package and import in std.experimental.allocator? Or perhaps even public? User code could benefit from that information.

Is it public in std.experimental.allocator? If so public, if not package and unify the definitions.

It is public.

thewilsonator · 2018-11-29T02:08:02Z

std/typecons.d

+    }
+
+    InstanceStorage!SafeClass buf;
+    auto support = (() @trusted => cast(SafeClass) (buf.ptr))();


If this is the intended usage of InstanceStorage it seems to me that it would be better done as a type safe layer over AlignedStorage than simply an alias.

std/typecons.d

radcapricorn · 2018-11-29T15:24:15Z

As far as convincing goes, there are already places in the standard library that use this kind of storage. Allocators is one, Scoped is the other. There may be more, this is just off the top of my head.

RazvanN7 · 2021-10-22T09:47:43Z

@andralex are you convinced about this addition?

@atilaneves What are your thoughts on this?

@radcapricorn you still need to fix some style issues for this to be ready to merge. Also, a changelog entry is required since you are adding new symbols.

andralex · 2021-10-22T20:16:28Z

I'm unclear about (a) what the use cases are, and (b) how fit this API is for those. Far as I can tell the main use case is "I want inert storage fit for one object of a given type". If that's the case, all we need is:

union AlignedStorage(T) 
{
    private T unused = void;
}

Maybe we could promote that as an idiom instead of putting it in stdlib because it's an advanced systems use?

For class instances, it seems more difficult so maybe that would be worth finding a good API for. But even in that case, don't all classes have the same alignment (same as a pointer)? So an array of void* sized appropriately would work fine.

atilaneves · 2021-10-25T11:04:36Z

@andralex are you convinced about this addition?

@atilaneves What are your thoughts on this?

@radcapricorn you still need to fix some style issues for this to be ready to merge. Also, a changelog entry is required since you are adding new symbols.

I don't think enough justification has been given for the inclusion of this in the standard library.

RazvanN7 · 2021-10-27T06:33:38Z

@andralex @atilaneves The argument given by @radcapricorn is that the invariant safeguards against misaligned moves.

atilaneves · 2021-11-03T14:01:44Z

@andralex @atilaneves The argument given by @radcapricorn is that the invariant safeguards against misaligned moves.

But why necessarily in the standard library?

RazvanN7 · 2021-11-04T12:58:42Z

I am going to close this as @radcapricorn seems to have abandoned it and @atilaneves and @andralex are against adding it to the standard library.

@radcapricorn please reopen if you ever turn back it.

CyberShadow reviewed May 25, 2017

View reviewed changes

std/typecons.d Outdated Show resolved Hide resolved

std/typecons.d Outdated Show resolved Hide resolved

JackStouffer added the Enhancement label May 25, 2017

MetaLang added the @andralex Approval from Andrei is required label Aug 6, 2017

andralex reviewed Aug 7, 2017

View reviewed changes

std/typecons.d Show resolved Hide resolved

std/typecons.d Show resolved Hide resolved

std/typecons.d Show resolved Hide resolved

std/typecons.d Show resolved Hide resolved

dlang-bot added stalled Needs Rebase labels Dec 29, 2017

thewilsonator reviewed Nov 25, 2018

View reviewed changes

std/typecons.d Outdated Show resolved Hide resolved

radcapricorn force-pushed the AlignedStorage branch from c1df322 to 1afacb2 Compare November 28, 2018 11:37

radcapricorn requested review from MetaLang and PetarKirov as code owners November 28, 2018 11:37

dlang-bot removed the Needs Rebase label Nov 28, 2018

dlang-bot removed the stalled label Nov 28, 2018

thewilsonator approved these changes Nov 28, 2018

View reviewed changes

radcapricorn force-pushed the AlignedStorage branch from 1afacb2 to 9eca834 Compare November 28, 2018 12:18

andralex reviewed Nov 28, 2018

View reviewed changes

thewilsonator reviewed Nov 29, 2018

View reviewed changes

std/typecons.d Show resolved Hide resolved

dlang-bot added the stalled label May 27, 2021

RazvanN7 added the Atila Neves label Oct 22, 2021

dlang-bot removed the stalled label Oct 22, 2021

RazvanN7 closed this Nov 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AlignedStorage and InstanceStorage types #5426

AlignedStorage and InstanceStorage types #5426

radcapricorn commented May 24, 2017 •

edited

radcapricorn commented May 24, 2017

radcapricorn commented May 25, 2017

MetaLang commented Aug 6, 2017

andralex left a comment

radcapricorn commented Nov 25, 2018

radcapricorn commented Nov 25, 2018

thewilsonator commented Nov 25, 2018

dlang-bot commented Nov 28, 2018

thewilsonator commented Nov 28, 2018

andralex left a comment

andralex Nov 28, 2018

radcapricorn Nov 28, 2018

andralex Nov 28, 2018

andralex Nov 28, 2018

radcapricorn Nov 28, 2018

thewilsonator Nov 29, 2018

radcapricorn Nov 29, 2018

thewilsonator Nov 29, 2018

radcapricorn commented Nov 29, 2018

RazvanN7 commented Oct 22, 2021 •

edited

andralex commented Oct 22, 2021

atilaneves commented Oct 25, 2021

RazvanN7 commented Oct 27, 2021

atilaneves commented Nov 3, 2021

RazvanN7 commented Nov 4, 2021

AlignedStorage and InstanceStorage types #5426

AlignedStorage and InstanceStorage types #5426

Conversation

radcapricorn commented May 24, 2017 • edited

radcapricorn commented May 24, 2017

radcapricorn commented May 25, 2017

MetaLang commented Aug 6, 2017

andralex left a comment

Choose a reason for hiding this comment

radcapricorn commented Nov 25, 2018

radcapricorn commented Nov 25, 2018

thewilsonator commented Nov 25, 2018

dlang-bot commented Nov 28, 2018

Bugzilla references

Testing this PR locally

thewilsonator commented Nov 28, 2018

andralex left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

radcapricorn commented Nov 29, 2018

RazvanN7 commented Oct 22, 2021 • edited

andralex commented Oct 22, 2021

atilaneves commented Oct 25, 2021

RazvanN7 commented Oct 27, 2021

atilaneves commented Nov 3, 2021

RazvanN7 commented Nov 4, 2021

radcapricorn commented May 24, 2017 •

edited

RazvanN7 commented Oct 22, 2021 •

edited