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.

Sign up for GitHub

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

Jump to bottom

Fix cycles in Phobos #4493

Merged
merged 1 commit into from
Jul 5, 2016
Merged

Fix cycles in Phobos #4493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion posix.mak
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ EXTRA_MODULES_INTERNAL := $(addprefix \
std/internal/digest/, sha_SSSE3 ) $(addprefix \
std/internal/math/, biguintcore biguintnoasm biguintx86 \
gammafunction errorfunction) $(addprefix std/internal/, \
cstring processinit unicode_tables scopebuffer\
cstring phobosinit unicode_tables scopebuffer\
unicode_comp unicode_decomp unicode_grapheme unicode_norm) \
$(addprefix std/internal/test/, dummyrange) \
$(addprefix std/experimental/ndslice/, internal) \
Expand Down
24 changes: 16 additions & 8 deletions std/encoding.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2635,10 +2635,11 @@ abstract class EncodingScheme
*/
class EncodingSchemeASCII : EncodingScheme
{
/* // moved to std.internal.phobosinit
shared static this()
{
Copy link
Member

Choose a reason for hiding this comment

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

Why not just delete these

Copy link
Member Author

Choose a reason for hiding this comment

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

I could if that's the consensus. The issue is that these really should go here. The example in the base class says to do it this way (and it will be fine for external libs to do that)

EncodingScheme.register("std.encoding.EncodingSchemeASCII");
}
}*/

const
{
Expand Down Expand Up @@ -2720,10 +2721,11 @@ class EncodingSchemeASCII : EncodingScheme
*/
class EncodingSchemeLatin1 : EncodingScheme
{
/* // moved to std.internal.phobosinit
shared static this()
{
EncodingScheme.register("std.encoding.EncodingSchemeLatin1");
}
}*/

const
{
Expand Down Expand Up @@ -2799,10 +2801,11 @@ class EncodingSchemeLatin1 : EncodingScheme
*/
class EncodingSchemeLatin2 : EncodingScheme
{
/* // moved to std.internal.phobosinit
shared static this()
{
EncodingScheme.register("std.encoding.EncodingSchemeLatin2");
}
}*/

const
{
Expand Down Expand Up @@ -2870,10 +2873,11 @@ class EncodingSchemeLatin2 : EncodingScheme
*/
class EncodingSchemeWindows1250 : EncodingScheme
{
/* // moved to std.internal.phobosinit
shared static this()
{
EncodingScheme.register("std.encoding.EncodingSchemeWindows1250");
}
}*/

const
{
Expand Down Expand Up @@ -2937,10 +2941,11 @@ class EncodingSchemeWindows1250 : EncodingScheme
*/
class EncodingSchemeWindows1252 : EncodingScheme
{
/* // moved to std.internal.phobosinit
shared static this()
{
EncodingScheme.register("std.encoding.EncodingSchemeWindows1252");
}
}*/

const
{
Expand Down Expand Up @@ -3004,10 +3009,11 @@ class EncodingSchemeWindows1252 : EncodingScheme
*/
class EncodingSchemeUtf8 : EncodingScheme
{
/* // moved to std.internal.phobosinit
shared static this()
{
EncodingScheme.register("std.encoding.EncodingSchemeUtf8");
}
}*/

const
{
Expand Down Expand Up @@ -3072,10 +3078,11 @@ class EncodingSchemeUtf8 : EncodingScheme
*/
class EncodingSchemeUtf16Native : EncodingScheme
{
/* // moved to std.internal.phobosinit
shared static this()
{
EncodingScheme.register("std.encoding.EncodingSchemeUtf16Native");
}
}*/

const
{
Expand Down Expand Up @@ -3167,10 +3174,11 @@ unittest
*/
class EncodingSchemeUtf32Native : EncodingScheme
{
/* // moved to std.internal.phobosinit
shared static this()
{
EncodingScheme.register("std.encoding.EncodingSchemeUtf32Native");
}
}*/

const
{
Expand Down
15 changes: 0 additions & 15 deletions std/experimental/allocator/building_blocks/region.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,21 +596,6 @@ version(Posix) struct SbrkRegion(uint minAlign = platformAlignment)
private static shared pthread_mutex_t sbrkMutex = PTHREAD_MUTEX_INITIALIZER;
import std.typecons : Ternary;

// workaround for https://issues.dlang.org/show_bug.cgi?id=14617
version(OSX)
{
shared static this()
{
pthread_mutex_init(cast(pthread_mutex_t*) &sbrkMutex, null) == 0
|| assert(0);
}
shared static ~this()
{
pthread_mutex_destroy(cast(pthread_mutex_t*) &sbrkMutex) == 0
|| assert(0);
}
}

static assert(minAlign.isGoodStaticAlignment);
static assert(size_t.sizeof == (void*).sizeof);
private shared void* _brkInitial, _brkCurrent;
Expand Down
35 changes: 35 additions & 0 deletions std/internal/phobosinit.d
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Written in the D programming language.

/++
The purpose of this module is to perform static construction away from the
normal modules to eliminate cyclic construction errors.

Copyright: Copyright 2011 - 2016
License: $(HTTP www.boost.org/LICENSE_1_0.txt, Boost License 1.0).
Authors: Jonathan M Davis, Kato Shoichi, Steven Schveighoffer
Source: $(PHOBOSSRC std/internal/_phobosinit.d)
+/
module std.internal.phobosinit;

version(OSX)
{
extern(C) void std_process_shared_static_this();

shared static this()
{
std_process_shared_static_this();
}
}

shared static this()
{
import std.encoding;
Copy link
Member

Choose a reason for hiding this comment

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

Lots of folks would like to avoid pulling in std.encoding at all

Copy link
Member Author

@schveiguy schveiguy Jun 28, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I have to say, the way this is done in std.encoding is horrific. -- You register the encoding class name by string, and then it uses the runtime to look up the class info via search of that string. THEN it constructs it, and checks all the encodings that are supported (and then lets the GC collect it).

There MUST be a better way to do this...

Copy link
Member

Choose a reason for hiding this comment

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

Oh, let's not forget that register isn't thread safe, because the AA holding the encoding schemes is __gshared

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, technically it doesn't matter for shared static this, since there is only one thread. But the fact that it's not protected at later times is bad

Copy link
Member

@JackStouffer JackStouffer Jun 29, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

register is designed to be called by user code as well as Phobos, there's no guarantee that the user code will be in a shared static this

EncodingScheme.register("std.encoding.EncodingSchemeASCII");
EncodingScheme.register("std.encoding.EncodingSchemeLatin1");
EncodingScheme.register("std.encoding.EncodingSchemeLatin2");
EncodingScheme.register("std.encoding.EncodingSchemeWindows1250");
EncodingScheme.register("std.encoding.EncodingSchemeWindows1252");
EncodingScheme.register("std.encoding.EncodingSchemeUtf8");
EncodingScheme.register("std.encoding.EncodingSchemeUtf16Native");
EncodingScheme.register("std.encoding.EncodingSchemeUtf32Native");
}
22 changes: 0 additions & 22 deletions std/internal/processinit.d
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion std/process.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ import std.conv;
import std.exception;
import std.path;
import std.stdio;
import std.internal.processinit;
import std.internal.cstring;


Expand Down
2 changes: 1 addition & 1 deletion win32.mak
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ SRC_STD_C_FREEBSD= \

SRC_STD_INTERNAL= \
std\internal\cstring.d \
std\internal\processinit.d \
std\internal\phobosinit.d \
std\internal\unicode_tables.d \
std\internal\unicode_comp.d \
std\internal\unicode_decomp.d \
Expand Down
2 changes: 1 addition & 1 deletion win64.mak
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ SRC_STD_C_FREEBSD= \

SRC_STD_INTERNAL= \
std\internal\cstring.d \
std\internal\processinit.d \
std\internal\phobosinit.d \
std\internal\unicode_tables.d \
std\internal\unicode_comp.d \
std\internal\unicode_decomp.d \
Expand Down