diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index ed27c241e..7716d6de5 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -24,8 +24,6 @@ jobs: # macOS - job_name: macOS 13 x64 os: macos-13 - - job_name: macOS 12 x64 - os: macos-12 # Windows - job_name: Windows x64 os: windows-2022 @@ -83,13 +81,6 @@ jobs: with: arch: ${{ env.MODEL == '64' && 'x64' || 'x86' }} - # NOTE: Linker ICEs with Xcode 15.0.1 (default version on macos-13) - # * https://issues.dlang.org/show_bug.cgi?id=24407 - # Remove this step if the default gets changed to 15.1 in actions/runner-images. - - name: 'macOS 13: Switch to Xcode v15.1' - if: matrix.os == 'macos-13' - run: sudo xcode-select -switch /Applications/Xcode_15.1.app - - name: 'Posix: Install host compiler' if: runner.os != 'Windows' run: cd ../dmd && ci/run.sh install_host_compiler diff --git a/std/algorithm/searching.d b/std/algorithm/searching.d index 42a9df518..b7119d24b 100644 --- a/std/algorithm/searching.d +++ b/std/algorithm/searching.d @@ -3735,6 +3735,47 @@ if (isInputRange!Range && !isInfinite!Range && assert(arr.minElement!"a.val".val == 0); } +// https://issues.dlang.org/show_bug.cgi?id=24827 +@safe unittest +{ + static struct S + { + int i; + bool destroyed; + + this(int i) @safe + { + this.i = i; + } + + ~this() @safe + { + destroyed = true; + } + + bool opEquals()(auto ref S rhs) + { + return this.i == rhs.i; + } + + int opCmp()(auto ref S rhs) + { + if (this.i < rhs.i) + return -1; + + return this.i == rhs.i ? 0 : 1; + } + + @safe invariant + { + assert(!destroyed); + } + } + + auto arr = [S(19), S(2), S(145), S(7)]; + assert(minElement(arr) == S(2)); +} + /** Iterates the passed range and returns the maximal element. A custom mapping function can be passed to `map`. @@ -3888,6 +3929,47 @@ if (isInputRange!Range && !isInfinite!Range && assert(arr[0].getI == 2); } +// https://issues.dlang.org/show_bug.cgi?id=24827 +@safe unittest +{ + static struct S + { + int i; + bool destroyed; + + this(int i) @safe + { + this.i = i; + } + + ~this() @safe + { + destroyed = true; + } + + bool opEquals()(auto ref S rhs) + { + return this.i == rhs.i; + } + + int opCmp()(auto ref S rhs) + { + if (this.i < rhs.i) + return -1; + + return this.i == rhs.i ? 0 : 1; + } + + @safe invariant + { + assert(!destroyed); + } + } + + auto arr = [S(19), S(2), S(145), S(7)]; + assert(maxElement(arr) == S(145)); +} + // minPos /** Computes a subrange of `range` starting at the first occurrence of `range`'s diff --git a/std/array.d b/std/array.d index acd5311c4..3313dbb28 100644 --- a/std/array.d +++ b/std/array.d @@ -3639,6 +3639,7 @@ if (isDynamicArray!A) } else { + import core.stdc.string : memcpy, memset; // Time to reallocate. // We need to almost duplicate what's in druntime, except we // have better access to the capacity field. @@ -3650,6 +3651,15 @@ if (isDynamicArray!A) if (u) { // extend worked, update the capacity + // if the type has indirections, we need to zero any new + // data that we requested, as the existing data may point + // at large unused blocks. + static if (hasIndirections!T) + { + immutable addedSize = u - (_data.capacity * T.sizeof); + () @trusted { memset(_data.arr.ptr + _data.capacity, 0, addedSize); }(); + } + _data.capacity = u / T.sizeof; return; } @@ -3665,10 +3675,20 @@ if (isDynamicArray!A) auto bi = (() @trusted => GC.qalloc(nbytes, blockAttribute!T))(); _data.capacity = bi.size / T.sizeof; - import core.stdc.string : memcpy; if (len) () @trusted { memcpy(bi.base, _data.arr.ptr, len * T.sizeof); }(); + _data.arr = (() @trusted => (cast(Unqual!T*) bi.base)[0 .. len])(); + + // we requested new bytes that are not in the existing + // data. If T has pointers, then this new data could point at stale + // objects from the last time this block was allocated. Zero that + // new data out, it may point at large unused blocks! + static if (hasIndirections!T) + () @trusted { + memset(bi.base + (len * T.sizeof), 0, (newlen - len) * T.sizeof); + }(); + _data.tryExtendBlock = true; // leave the old data, for safety reasons } @@ -4047,6 +4067,43 @@ if (isDynamicArray!A) app2.toString(); } +// https://issues.dlang.org/show_bug.cgi?id=24856 +@system unittest +{ + import core.memory : GC; + import std.stdio : writeln; + import std.algorithm.searching : canFind; + GC.disable(); + scope(exit) GC.enable(); + void*[] freeme; + // generate some poison blocks to allocate from. + auto poison = cast(void*) 0xdeadbeef; + foreach (i; 0 .. 10) + { + auto blk = new void*[7]; + blk[] = poison; + freeme ~= blk.ptr; + } + + foreach (p; freeme) + GC.free(p); + + int tests = 0; + foreach (i; 0 .. 10) + { + Appender!(void*[]) app; + app.put(null); + // if not a realloc of one of the deadbeef pointers, continue + if (!freeme.canFind(app.data.ptr)) + continue; + ++tests; + assert(!app.data.ptr[0 .. app.capacity].canFind(poison), "Appender not zeroing data!"); + } + // just notify in the log whether this test actually could be done. + if (tests == 0) + writeln("WARNING: test of Appender zeroing did not occur"); +} + //Calculates an efficient growth scheme based on the old capacity //of data, and the minimum requested capacity. //arg curLen: The current length diff --git a/std/bitmanip.d b/std/bitmanip.d index 15211a3a9..f8a97dfbf 100644 --- a/std/bitmanip.d +++ b/std/bitmanip.d @@ -106,7 +106,7 @@ private template createAccessors( enum RightShiftOp = ">>>="; } - static if (is(T == bool)) + static if (is(T : bool)) { enum createAccessors = // getter @@ -4676,3 +4676,24 @@ if (isIntegral!T) foreach (i; 0 .. 63) assert(bitsSet(1UL << i).equal([i])); } + +// Fix https://issues.dlang.org/show_bug.cgi?id=24095 +@safe @nogc pure unittest +{ + enum Bar : bool + { + a, + b, + } + + struct Foo + { + mixin(bitfields!(Bar, "bar", 1, ubyte, "", 7,)); + } + + Foo foo; + foo.bar = Bar.a; + assert(foo.bar == Bar.a); + foo.bar = Bar.b; + assert(foo.bar == Bar.b); +} diff --git a/std/container/dlist.d b/std/container/dlist.d index 728aacde5..8f7df105c 100644 --- a/std/container/dlist.d +++ b/std/container/dlist.d @@ -185,6 +185,7 @@ Implements a doubly-linked list. struct DList(T) { import std.range : Take; + import std.traits : isMutable; /* A Node with a Payload. A PayNode. @@ -220,7 +221,10 @@ struct DList(T) { import std.algorithm.mutation : move; - return (new PayNode(BaseNode(prev, next), move(arg))).asBaseNode(); + static if (isMutable!Stuff) + return (new PayNode(BaseNode(prev, next), move(arg))).asBaseNode(); + else + return (new PayNode(BaseNode(prev, next), arg)).asBaseNode(); } void initialize() nothrow @safe pure @@ -1149,3 +1153,22 @@ private: list.removeFront(); assert(list[].walkLength == 0); } + +// https://issues.dlang.org/show_bug.cgi?id=24637 +@safe unittest +{ + import std.algorithm.comparison : equal; + + struct A + { + int c; + } + + DList!A B; + B.insert(A(1)); + assert(B[].equal([A(1)])); + + const a = A(3); + B.insert(a); + assert(B[].equal([A(1), A(3)])); +} diff --git a/std/logger/core.d b/std/logger/core.d index cc938d4fd..1e879fd6c 100644 --- a/std/logger/core.d +++ b/std/logger/core.d @@ -1433,7 +1433,7 @@ logger by the user, the default logger's log level is LogLevel.info. Example: ------------- -sharedLog = new FileLogger(yourFile); +sharedLog = new shared FileLogger(yourFile); ------------- The example sets a new `FileLogger` as new `sharedLog`. @@ -1450,7 +1450,7 @@ writing `sharedLog`. The default `Logger` is thread-safe. ------------- if (sharedLog !is myLogger) - sharedLog = new myLogger; + sharedLog = new shared myLogger; ------------- */ @property shared(Logger) sharedLog() @safe diff --git a/std/logger/filelogger.d b/std/logger/filelogger.d index c662ca74e..5ba167c7b 100644 --- a/std/logger/filelogger.d +++ b/std/logger/filelogger.d @@ -37,7 +37,7 @@ class FileLogger : Logger auto l3 = new FileLogger("logFile", LogLevel.fatal, CreateFolder.yes); ------------- */ - this(const string fn, const LogLevel lv = LogLevel.all) @safe + this(this This)(const string fn, const LogLevel lv = LogLevel.all) { this(fn, lv, CreateFolder.yes); } @@ -63,7 +63,7 @@ class FileLogger : Logger auto l2 = new FileLogger(file, LogLevel.fatal); ------------- */ - this(const string fn, const LogLevel lv, CreateFolder createFileNameFolder) @safe + this(this This)(const string fn, const LogLevel lv, CreateFolder createFileNameFolder) { import std.file : exists, mkdirRecurse; import std.path : dirName; @@ -80,7 +80,8 @@ class FileLogger : Logger " created in '", d,"' could not be created.")); } - this.file_.open(this.filename, "a"); + // Cast away `shared` when the constructor is inferred shared. + () @trusted { (cast() this.file_).open(this.filename, "a"); }(); } /** A constructor for the `FileLogger` Logger that takes a reference to @@ -270,3 +271,12 @@ class FileLogger : Logger assert(tl !is null); stdThreadLocalLog.logLevel = LogLevel.all; } + +@safe unittest +{ + // we don't need to actually run the code, only make sure + // it compiles + static _() { + auto l = new shared FileLogger(""); + } +} diff --git a/std/logger/package.d b/std/logger/package.d index 14a439486..215ca20b8 100644 --- a/std/logger/package.d +++ b/std/logger/package.d @@ -64,7 +64,7 @@ using the property called `sharedLog`. This property is a reference to the current default `Logger`. This reference can be used to assign a new default `Logger`. ------------- -sharedLog = new FileLogger("New_Default_Log_File.log"); +sharedLog = new shared FileLogger("New_Default_Log_File.log"); ------------- Additional `Logger` can be created by creating a new instance of the diff --git a/std/process.d b/std/process.d index 4f593bd06..2efbcaa84 100644 --- a/std/process.d +++ b/std/process.d @@ -4631,11 +4631,12 @@ else version (Posix) if (childpid == 0) { // Trusted because args and all entries are always zero-terminated - (() @trusted => - core.sys.posix.unistd.execvp(args[0], &args[0]) || - perror(args[0]) // failed to execute - )(); - return; + (() @trusted { + core.sys.posix.unistd.execvp(args[0], &args[0]); + perror(args[0]); + core.sys.posix.unistd._exit(1); + })(); + assert(0, "Child failed to exec"); } if (browser) // Trusted because it's allocated via strdup above diff --git a/std/traits.d b/std/traits.d index 69362c086..3c227278c 100644 --- a/std/traits.d +++ b/std/traits.d @@ -9238,12 +9238,16 @@ enum isCopyable(S) = __traits(isCopyable, S); * is the same as `T`. For pointer and slice types, it is `T` with the * outer-most layer of qualifiers dropped. */ -package(std) template DeducedParameterType(T) +package(std) alias DeducedParameterType(T) = DeducedParameterTypeImpl!T; +/// ditto +package(std) alias DeducedParameterType(alias T) = DeducedParameterTypeImpl!T; + +private template DeducedParameterTypeImpl(T) { static if (is(T == U*, U) || is(T == U[], U)) - alias DeducedParameterType = Unqual!T; + alias DeducedParameterTypeImpl = Unqual!T; else - alias DeducedParameterType = T; + alias DeducedParameterTypeImpl = T; } @safe unittest @@ -9263,6 +9267,7 @@ package(std) template DeducedParameterType(T) } static assert(is(DeducedParameterType!NoCopy == NoCopy)); + static assert(is(DeducedParameterType!(inout(NoCopy)) == inout(NoCopy))); } @safe unittest diff --git a/std/typecons.d b/std/typecons.d index c874c0ff8..bd462f53a 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -3104,17 +3104,18 @@ private: { static if (useQualifierCast) { - this.data = cast() value; + static if (hasElaborateAssign!T) + { + import core.lifetime : copyEmplace; + copyEmplace(cast() value, this.data); + } + else + this.data = cast() value; } else { - // As we're escaping a copy of `value`, deliberately leak a copy: - static union DontCallDestructor - { - T value; - } - DontCallDestructor copy = DontCallDestructor(value); - this.data = *cast(Payload*) © + import core.lifetime : copyEmplace; + copyEmplace(cast() value, cast() *cast(T*) &this.data); } } @@ -3139,6 +3140,334 @@ package(std) Rebindable2!T rebindable2(T)(T value) return Rebindable2!T(value); } +// Verify that the destructor is called properly if there is one. +@system unittest +{ + { + bool destroyed; + + struct S + { + int i; + + this(int i) @safe + { + this.i = i; + } + + ~this() @safe + { + destroyed = true; + } + } + + { + auto foo = rebindable2(S(42)); + + // Whether destruction has occurred here depends on whether the + // temporary gets moved or not, so we won't assume that it has or + // hasn't happened. What we care about here is that foo gets destroyed + // properly when it leaves the scope. + destroyed = false; + } + assert(destroyed); + + { + auto foo = rebindable2(const S(42)); + destroyed = false; + } + assert(destroyed); + } + + // Test for double destruction with qualifer cast being used + { + static struct S + { + int i; + bool destroyed; + + this(int i) @safe + { + this.i = i; + } + + ~this() @safe + { + destroyed = true; + } + + @safe invariant + { + assert(!destroyed); + } + } + + { + auto foo = rebindable2(S(42)); + assert(typeof(foo).useQualifierCast); + assert(foo.data.i == 42); + assert(!foo.data.destroyed); + } + { + auto foo = rebindable2(S(42)); + destroy(foo); + } + { + auto foo = rebindable2(const S(42)); + assert(typeof(foo).useQualifierCast); + assert(foo.data.i == 42); + assert(!foo.data.destroyed); + } + { + auto foo = rebindable2(const S(42)); + destroy(foo); + } + } + + // Test for double destruction without qualifer cast being used + { + static struct S + { + int i; + bool destroyed; + + this(int i) @safe + { + this.i = i; + } + + ~this() @safe + { + destroyed = true; + } + + @disable ref S opAssign()(auto ref S rhs); + + @safe invariant + { + assert(!destroyed); + } + } + + { + auto foo = rebindable2(S(42)); + assert(!typeof(foo).useQualifierCast); + assert((cast(S*)&(foo.data)).i == 42); + assert(!(cast(S*)&(foo.data)).destroyed); + } + { + auto foo = rebindable2(S(42)); + destroy(foo); + } + } +} + +// Verify that if there is an overloaded assignment operator, it's not assigned +// to garbage. +@safe unittest +{ + static struct S + { + int i; + bool destroyed; + + this(int i) @safe + { + this.i = i; + } + + ~this() @safe + { + destroyed = true; + } + + ref opAssign()(auto ref S rhs) + { + assert(!this.destroyed); + this.i = rhs.i; + return this; + } + } + + { + auto foo = rebindable2(S(42)); + foo = S(99); + assert(foo.data.i == 99); + } + { + auto foo = rebindable2(S(42)); + foo = const S(99); + assert(foo.data.i == 99); + } +} + +// Verify that postblit or copy constructor is called properly if there is one. +@system unittest +{ + // postblit with type qualifier cast + { + static struct S + { + int i; + static bool copied; + + this(this) @safe + { + copied = true; + } + } + + { + auto foo = rebindable2(S(42)); + + // Whether a copy has occurred here depends on whether the + // temporary gets moved or not, so we won't assume that it has or + // hasn't happened. What we care about here is that foo gets copied + // properly when we copy it below. + S.copied = false; + + auto bar = foo; + assert(S.copied); + } + { + auto foo = rebindable2(const S(42)); + assert(typeof(foo).useQualifierCast); + S.copied = false; + + auto bar = foo; + assert(S.copied); + } + } + + // copy constructor with type qualifier cast + { + static struct S + { + int i; + static bool copied; + + this(ref inout S rhs) @safe inout + { + this.i = i; + copied = true; + } + } + + { + auto foo = rebindable2(S(42)); + assert(typeof(foo).useQualifierCast); + S.copied = false; + + auto bar = foo; + assert(S.copied); + } + { + auto foo = rebindable2(const S(42)); + S.copied = false; + + auto bar = foo; + assert(S.copied); + } + } + + // FIXME https://issues.dlang.org/show_bug.cgi?id=24829 + + // Making this work requires either reworking how the !useQualiferCast + // version works so that the compiler can correctly generate postblit + // constructors and copy constructors as appropriate, or an explicit + // postblit or copy constructor needs to be added for such cases, which + // gets pretty complicated if we want to correctly add the same attributes + // that T's postblit or copy constructor has. + + /+ + // postblit without type qualifier cast + { + static struct S + { + int* ptr; + static bool copied; + + this(int i) + { + ptr = new int(i); + } + + this(this) @safe + { + if (ptr !is null) + ptr = new int(*ptr); + copied = true; + } + + @disable ref S opAssign()(auto ref S rhs); + } + + { + auto foo = rebindable2(S(42)); + assert(!typeof(foo).useQualifierCast); + S.copied = false; + + auto bar = foo; + assert(S.copied); + assert(*(cast(S*)&(foo.data)).ptr == *(cast(S*)&(bar.data)).ptr); + assert((cast(S*)&(foo.data)).ptr !is (cast(S*)&(bar.data)).ptr); + } + { + auto foo = rebindable2(const S(42)); + S.copied = false; + + auto bar = foo; + assert(S.copied); + assert(*(cast(S*)&(foo.data)).ptr == *(cast(S*)&(bar.data)).ptr); + assert((cast(S*)&(foo.data)).ptr !is (cast(S*)&(bar.data)).ptr); + } + } + + // copy constructor without type qualifier cast + { + static struct S + { + int* ptr; + static bool copied; + + this(int i) + { + ptr = new int(i); + } + + this(ref inout S rhs) @safe inout + { + if (rhs.ptr !is null) + ptr = new inout int(*rhs.ptr); + copied = true; + } + + @disable ref S opAssign()(auto ref S rhs); + } + + { + auto foo = rebindable2(S(42)); + assert(!typeof(foo).useQualifierCast); + S.copied = false; + + auto bar = foo; + assert(S.copied); + assert(*(cast(S*)&(foo.data)).ptr == *(cast(S*)&(bar.data)).ptr); + assert((cast(S*)&(foo.data)).ptr !is (cast(S*)&(bar.data)).ptr); + } + { + auto foo = rebindable2(const S(42)); + S.copied = false; + + auto bar = foo; + assert(S.copied); + assert(*(cast(S*)&(foo.data)).ptr == *(cast(S*)&(bar.data)).ptr); + assert((cast(S*)&(foo.data)).ptr !is (cast(S*)&(bar.data)).ptr); + } + } + +/ +} + /** Similar to `Rebindable!(T)` but strips all qualifiers from the reference as opposed to just constness / immutability. Primary intended use case is with