Closed
Bug 1407544
Opened 7 years ago
Closed 7 years ago
Add a helper class for manual construction/destruction of (static) instances
Categories
(Core :: MFBT, enhancement, P3)
Core
MFBT
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: glandium, Assigned: glandium)
Details
In mozjemalloc, the initialization ordering constraints are such that many static variables can't be statically initialized (some allocator static initializer might run after some other static initializer that needs the allocator to be fully initialized), and instead, they need to be manually initialized (and we don't care about their destructors ever running).
js/src/threading/ExclusiveData.h, which, relatedly, I'd like to move to mozglue so that it lives next to PlatformMutex and PlatformConditionVariable, abuses AlignedStorage2 to achieve the same kind of effect, using placement new and calling the destructor manually.
I'm however in search for a name for such a helper class. Any ideas?
Flags: needinfo?(nfroyd)
Flags: needinfo?(jwalden+bmo)
Comment 2•7 years ago
|
||
(In reply to David Major [:dmajor] from comment #1)
> I'm curious why Maybe<T> doesn't work for this use case?
I assume Maybe<T> is not desirable here because it causes static constructors, if only to register the non-trivial destructor, which we don't care about running per comment 0.
Maybe ManuallyInitialized<T>? StorageFor<T>? I'm unsure of what this class would achieve over using AlignedStorage2. Maybe the name would be more clear, but it would basically be a thin wrapper over AlignedStorage2, perhaps with an emplace() method for calling the constructor of T? Is there any reason we wouldn't want to add such an emplace() method to AlignedStorage2?
Flags: needinfo?(nfroyd)
Comment 3•7 years ago
|
||
AlignedStorage{,2} has that problem that it's easy to copy it around, perhaps unintentionally, and invoke undefined behavior, so it's on the chopping block as http://whereswalden.com/2017/02/27/a-pitfall-in-c-low-level-object-creation-and-storage-and-how-to-avoid-it/ noted. That post also argues that this stuff is sufficiently low-level, and the use cases sufficiently infrequent/esoteric/expert-level, that it's preferable to spell things out manually without a helper class, so you're forced to spell out (and hopefully think about) the actual low-level things you're using and doing.
I think I continue to favor spelling things out, without a helper class. You're already off in the weeds a ways -- having to define that exact reality every time seems appropriately sobering.
Flags: needinfo?(jwalden+bmo)
Comment 4•7 years ago
|
||
(In reply to Jeff Walden [:Waldo] (I'm baaaaaaack...) from comment #3)
> AlignedStorage{,2} has that problem that it's easy to copy it around,
> perhaps unintentionally, and invoke undefined behavior...
We deleted copy construction/assignment for AlignedStorage2, though. So I am unsure what "copy it around" here means.
Assignee | ||
Comment 5•7 years ago
|
||
I guess I could try to use AlignedStorage2 with placement new, then.
Relatedly, is there a reason to keep it named AlignedStorage2, after bug 1341951?
Updated•7 years ago
|
Priority: -- → P3
Assignee | ||
Comment 6•7 years ago
|
||
I won't be using this in the end. Or will use AlignedStorage2 if I need something like this in the end.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•