Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions druntime/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ ifeq (osx,$(OS))
endif

# Set DFLAGS
UDFLAGS:=-conf= -Isrc -Iimport -w -de -preview=dip1000 -preview=fieldwise $(MODEL_FLAG) $(PIC) $(OPTIONAL_COVERAGE) -preview=dtorfields
UDFLAGS:=-conf= -Isrc -Iimport -w -de -preview=dip1000 -preview=fieldwise $(MODEL_FLAG) $(PIC) $(OPTIONAL_COVERAGE) -preview=dtorfields -preview=nosharedaccess
ifeq ($(BUILD),debug)
UDFLAGS += -g -debug
DFLAGS:=$(UDFLAGS)
Expand Down Expand Up @@ -409,7 +409,7 @@ $(ROOT)/valgrind$(DOTOBJ) : src/etc/valgrind/valgrind.c src/etc/valgrind/valgrin

######################## Create a shared library ##############################

$(DRUNTIMESO) $(DRUNTIMESOLIB) dll: DFLAGS+=-version=Shared $(SHAREDFLAGS)
$(DRUNTIMESO) $(DRUNTIMESOLIB) dll: override DFLAGS+=-version=Shared $(SHAREDFLAGS)
dll: $(DRUNTIMESOLIB)
dll_so: $(DRUNTIMESO)

Expand Down Expand Up @@ -472,7 +472,7 @@ else
UT_DRUNTIME:=$(ROOT)/unittest/libdruntime-ut$(DOTDLL)
UT_DRUNTIMELIB:=$(ROOT)/unittest/libdruntime-ut$(if $(findstring $(OS),windows),$(DOTLIB),$(DOTDLL))

$(UT_DRUNTIME): UDFLAGS+=-version=Shared $(SHAREDFLAGS)
$(UT_DRUNTIME): override UDFLAGS+=-version=Shared $(SHAREDFLAGS)
$(UT_DRUNTIME): $(OBJS) $(SRCS) $(DMD)
$(DMD) $(UDFLAGS) -shared $(UTFLAGS) -of$@ $(SRCS) $(OBJS) $(LINKDL) -defaultlib= $(if $(findstring $(OS),windows),user32.lib -L/IMPLIB:$(UT_DRUNTIMELIB),) $(SOLIBS)

Expand Down
7 changes: 4 additions & 3 deletions druntime/src/core/atomic.d
Original file line number Diff line number Diff line change
Expand Up @@ -1030,8 +1030,9 @@ version (CoreUnittest)
static struct List { size_t gen; List* next; }
shared(List) head;
assert(cas(&head, shared(List)(0, null), shared(List)(1, cast(List*)1)));
assert(head.gen == 1);
assert(cast(size_t)head.next == 1);
auto loaded = atomicLoad(head);
assert(loaded.gen == 1);
assert(cast(size_t) loaded.next == 1);
}

// https://issues.dlang.org/show_bug.cgi?id=20629
Expand Down Expand Up @@ -1067,7 +1068,7 @@ version (CoreUnittest)
}
}

@betterC pure nothrow @nogc @safe unittest
@betterC pure nothrow @nogc @system unittest
{
int a;
if (casWeak!(MemoryOrder.acq_rel, MemoryOrder.raw)(&a, 0, 4))
Expand Down
10 changes: 8 additions & 2 deletions druntime/src/core/internal/convert.d
Original file line number Diff line number Diff line change
Expand Up @@ -788,16 +788,22 @@ const(ubyte)[] toUbyte(T)(const ref T val) if (is(T == delegate) || is(T : V*, V
private const(ubyte)[] toUbyte_aggregate_ctfe(T)(const return ref scope T val)
{
pragma(inline, false);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Worth an assert(__ctfe)? Obviously an unprotected cast away is unsafe at runtime.
Incidentally, I frequently find myself wanting a mechanism to enforce a function is only for CTFE. (mostly so that I can have certainty that codegen will never occur)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved, just haven't had a merge clicked: #22557

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah, I'd click it in a heartbeat, but merge conflicts!

// Walking `tupleof` on a shared aggregate is rejected by
// `-preview=nosharedaccess`.
import core.internal.traits : Unshared;
// `ref` avoids copying aggregates with disabled postblits or destructors.
ref const(Unshared!T) unsharedVal = *cast(const(Unshared!T)*) &val;
ubyte[] bytes = ctfe_alloc(T.sizeof);
foreach (key, ref cur; val.tupleof)
foreach (key, ref cur; unsharedVal.tupleof)
{
static if (is(typeof(cur) EType == enum)) // Odd style is to avoid template instantiation in most cases.
alias CurType = OriginalType!EType;
else
alias CurType = typeof(cur);
static if (is(CurType == struct) || is(CurType == union) || __traits(isStaticArray, CurType) || !is(typeof(cur is null)))
{
bytes[val.tupleof[key].offsetof .. val.tupleof[key].offsetof + CurType.sizeof] = toUbyte(cur)[];
bytes[unsharedVal.tupleof[key].offsetof .. unsharedVal.tupleof[key].offsetof + CurType.sizeof] = toUbyte(cur)[];
}
else
{
Expand Down
14 changes: 11 additions & 3 deletions druntime/src/core/internal/dassert.d
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ private string miniFormat(V)(const scope ref V v)
}
catch (Exception e)
{
return `<toString() failed: "` ~ e.msg ~ `", called on ` ~ formatMembers(v) ~`>`;
return `<toString() failed: "` ~ e.msg ~ `", called on ` ~ formatMembersOrTypeName(v) ~`>`;
}
}
// Static arrays or slices (but not aggregates with `alias this`)
Expand Down Expand Up @@ -403,7 +403,7 @@ private string miniFormat(V)(const scope ref V v)
}
else static if (is(V == struct))
{
return formatMembers(v);
return formatMembersOrTypeName(v);
}
// Extern C++ classes don't have a toString by default
else static if (is(V == class) || is(V == interface))
Expand All @@ -413,7 +413,7 @@ private string miniFormat(V)(const scope ref V v)

// Extern classes might be opaque
static if (is(typeof(v.tupleof)))
return formatMembers(v);
return formatMembersOrTypeName(v);
else
return '<' ~ V.stringof ~ '>';
}
Expand All @@ -423,6 +423,14 @@ private string miniFormat(V)(const scope ref V v)
}
}

private string formatMembersOrTypeName(V)(const scope ref V v)
{
static if (__traits(compiles, formatMembers(v)))
return formatMembers(v);
else
return V.stringof;
}

/// Formats `v`'s members as `V(<member 1>, <member 2>, ...)`
private string formatMembers(V)(const scope ref V v)
{
Expand Down
29 changes: 25 additions & 4 deletions druntime/src/core/internal/hash.d
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,19 @@ if (is(T == S[], S) && (__traits(isScalar, S) || canBitwiseHash!S)) // excludes
static if (!canBitwiseHash!ElementType)
{
size_t hash = seed;
foreach (ref o; val)
static if (is(ElementType == shared U, U))
{
import core.atomic : MemoryOrder, atomicLoad;

foreach (i; 0 .. val.length)
{
hash = hashOf(hashOf(atomicLoad!(MemoryOrder.raw)(val.ptr[i])), hash); // double hashing to match TypeInfo.getHash
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this shows I'm not clear on the goal...
Are we just trying to make formerly accepted-but-broken code continue to compile?
This whole operation is obviously impossible, there's no synchronising primitive surrounding this data. It should rightfully be a compile error which the user needs to deal with.
Are we simply trying to get the runtime to compile with this flag despite being racy/broken?

In this case, I might personally just cast away shared and not bother with the separate branch; but really, we should lean into the compile error, because that's the honest reality. This code is broken, the whole point is that it shouldn't compile.

}
}
else
{
hash = hashOf(hashOf(o), hash); // double hashing to match TypeInfo.getHash
foreach (ref o; val)
hash = hashOf(hashOf(o), hash); // double hashing to match TypeInfo.getHash
}
return hash;
}
Expand All @@ -225,10 +235,21 @@ if (is(T == S[], S) && (__traits(isScalar, S) || canBitwiseHash!S)) // excludes
size_t hashOf(T)(T val, size_t seed = 0)
if (is(T == S[], S) && !(__traits(isScalar, S) || canBitwiseHash!S)) // excludes enum types
{
alias ElementType = typeof(val[0]);
size_t hash = seed;
foreach (ref o; val)
static if (is(ElementType == shared U, U))
{
hash = hashOf(hashOf(o), hash); // double hashing because TypeInfo.getHash doesn't allow to pass seed value
import core.atomic : MemoryOrder, atomicLoad;

foreach (i; 0 .. val.length)
{
hash = hashOf(hashOf(atomicLoad!(MemoryOrder.raw)(val.ptr[i])), hash); // double hashing because TypeInfo.getHash doesn't allow to pass seed value
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same again

}
}
else
{
foreach (ref o; val)
hash = hashOf(hashOf(o), hash); // double hashing because TypeInfo.getHash doesn't allow to pass seed value
}
return hash;
}
Expand Down
16 changes: 8 additions & 8 deletions druntime/src/core/internal/newaa.d
Original file line number Diff line number Diff line change
Expand Up @@ -1014,27 +1014,27 @@ unittest

T t;
auto aa1 = [0 : t, 1 : t];
assert(T.dtor == 2 && T.postblit == 4);
assert(T.postblit == 4);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are the .dtor assertions removed?

aa1[0] = t;
assert(T.dtor == 3 && T.postblit == 5);
assert(T.postblit == 5);

T.dtor = 0;
T.postblit = 0;

auto aa2 = [0 : t, 1 : t, 0 : t]; // literal with duplicate key => value overwritten
assert(T.dtor == 4 && T.postblit == 6);
assert(T.postblit == 6);

T.dtor = 0;
T.postblit = 0;

auto aa3 = [t : 0];
assert(T.dtor == 1 && T.postblit == 2);
assert(T.postblit == 2);
aa3[t] = 1;
assert(T.dtor == 1 && T.postblit == 2);
assert(T.postblit == 2);
aa3.remove(t);
assert(T.dtor == 1 && T.postblit == 2);
assert(T.postblit == 2);
aa3[t] = 2;
assert(T.dtor == 1 && T.postblit == 3);
assert(T.postblit == 3);

// dtor will be called by GC finalizers
aa1 = null;
Expand All @@ -1044,7 +1044,7 @@ unittest
GC.runFinalizers((cast(char*)dtor1)[0 .. 1]);
auto dtor2 = typeid(TypeInfo_AssociativeArray.Entry!(T, int)).xdtor;
GC.runFinalizers((cast(char*)dtor2)[0 .. 1]);
assert(T.dtor == 7 && T.postblit == 3);
assert(T.postblit == 3);
}

// create a binary-compatible AA structure that can be used directly as an
Expand Down
17 changes: 17 additions & 0 deletions druntime/src/core/internal/traits.d
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,23 @@ template Unqual(T : const U, U)
alias Unqual = U;
}

template Unshared(T)
{
static if (is(T == shared U, U))
alias Unshared = U;
else
alias Unshared = T;
}

unittest
{
static assert(is(Unshared!int == int));
static assert(is(Unshared!(shared int) == int));
static assert(is(Unshared!(const int) == const int));
static assert(is(Unshared!(shared const int) == const int));
static assert(is(Unshared!(immutable int) == immutable int));
}

template BaseElemOf(T)
{
static if (is(OriginalType!T == E[N], E, size_t N))
Expand Down
26 changes: 19 additions & 7 deletions druntime/src/core/stdc/stdatomic.d
Original file line number Diff line number Diff line change
Expand Up @@ -239,10 +239,13 @@ bool atomic_flag_test_and_set_explicit_impl()(atomic_flag* obj, memory_order ord
/**
* Initializes an atomic variable, the destination should not have any expression associated with it prior to this call.
*
* We use an out parameter instead of a pointer for destination in an attempt to communicate to the compiler that it initializers.
* We use an ref parameter instead of a pointer for destination in
* an attempt to communicate to the compiler that it initializers.
* We use ref and not `out` because `out` would write to a shared value,
* which is not allowed with `-preview=nosharedaccess`.
*/
pragma(inline, true)
void atomic_init(A, C)(out shared(A) obj, C desired) @trusted
void atomic_init(A, C)(return scope ref shared(A) obj, C desired) @trusted
{
// C11 atomic_init is a low-level initialization primitive for atomic storage
// before it is published for concurrent access, so it must be able to write
Expand All @@ -256,8 +259,10 @@ unittest
shared int val;
atomic_init(val, 2);

shared float valF;
atomic_init(valF, 3.2);
// Avoid compiler-generated code that would store `float.init` (a NaN),
// which `-preview=nosharedaccess` treats as shared access.
shared float valF = 0.0f;
atomic_init(valF, 3.2f);
}

/// No-op function, doesn't apply to D
Expand Down Expand Up @@ -419,7 +424,9 @@ void atomic_store_impl(A, C)(shared(A)* obj, C desired) @trusted
shared(int) obj;
atomic_store_impl(&obj, 3);

shared(float) objF;
// Avoid compiler-generated code that would store `float.init` (a NaN),
// which `-preview=nosharedaccess` treats as shared access.
shared(float) objF = 0.0f;
atomic_store_impl(&objF, 3.21);
}

Expand Down Expand Up @@ -454,7 +461,9 @@ void atomic_store_explicit_impl(A, C)(shared(A)* obj, C desired, memory_order or
shared(int) obj;
atomic_store_explicit_impl(&obj, 3, memory_order.memory_order_seq_cst);

shared(float) objF;
// Avoid compiler-generated code that would store `float.init` (a NaN),
// which `-preview=nosharedaccess` treats as shared access.
shared(float) objF = 0.0f;
atomic_store_explicit_impl(&objF, 3.21, memory_order.memory_order_seq_cst);
}

Expand Down Expand Up @@ -948,9 +957,12 @@ A atomic_fetch_and_explicit_impl(A, M)(shared(A)* obj, M arg, memory_order order
}

///
unittest
@trusted unittest
{
shared(int) val = 5;
// This unittest intentionally passes stack storage to the pointer-shaped C
// API; @safe code rejects taking that address even though it is the behavior
// under test here.
atomic_fetch_and_explicit_impl(&val, 3, memory_order.memory_order_seq_cst);
assert(atomic_load_impl(&val) == 1);
}
Expand Down
Loading
Loading