From 12a49aa3a58e495a37cb4579b5cf5f7f50230ae4 Mon Sep 17 00:00:00 2001 From: Walter Bright Date: Tue, 16 Apr 2024 11:35:29 -0700 Subject: [PATCH] compile druntime with -preview=dip1021 --- compiler/src/dmd/escape.d | 89 +++++++++++++++++++++-- compiler/test/fail_compilation/fob1.d | 2 +- compiler/test/fail_compilation/fob2.d | 5 +- compiler/test/fail_compilation/test1021.d | 47 +++++++++--- druntime/Makefile | 2 +- druntime/src/core/demangle.d | 9 ++- druntime/src/core/internal/postblit.d | 3 +- druntime/src/core/lifetime.d | 7 +- druntime/src/object.d | 8 +- druntime/test/common.mak | 2 +- 10 files changed, 140 insertions(+), 34 deletions(-) diff --git a/compiler/src/dmd/escape.d b/compiler/src/dmd/escape.d index 4e1bc59987e4..89eca366b3d5 100644 --- a/compiler/src/dmd/escape.d +++ b/compiler/src/dmd/escape.d @@ -87,12 +87,15 @@ bool checkMutableArguments(Scope* sc, FuncDeclaration fd, TypeFunction tf, */ VarDeclaration[] outerVars = fd ? fd.outerVars[] : null; - const len = arguments.length + (ethis !is null) + outerVars.length; + //printf("len = %d outerVars.length = %d\n", cast(int)(*arguments).length, cast(int)outerVars.length); + const len = (*arguments).length + (ethis !is null) + outerVars.length; if (len <= 1) return errors; struct EscapeBy { + Loc loc; + Expression exp; EscapeByResults er; Parameter param; // null if no Parameter for this argument bool isMutable; // true if reference to mutable @@ -109,6 +112,8 @@ bool checkMutableArguments(Scope* sc, FuncDeclaration fd, TypeFunction tf, if (i < arguments.length) { arg = (*arguments)[i]; + eb.exp = arg; + eb.loc = arg.loc; if (i < paramLength) { eb.param = tf.parameterList[i]; @@ -129,6 +134,8 @@ bool checkMutableArguments(Scope* sc, FuncDeclaration fd, TypeFunction tf, */ eb.param = null; arg = ethis; + eb.exp = arg; + eb.loc = arg.loc; auto ad = fd.isThis(); assert(ad); assert(ethis); @@ -150,6 +157,7 @@ bool checkMutableArguments(Scope* sc, FuncDeclaration fd, TypeFunction tf, eb.param = null; refs = true; auto var = outerVars[i - (len - outerVars.length)]; + eb.loc = var.loc; eb.isMutable = var.type.isMutable(); eb.er.pushRef(var, false); continue; @@ -161,10 +169,14 @@ bool checkMutableArguments(Scope* sc, FuncDeclaration fd, TypeFunction tf, escapeByValue(arg, &eb.er); } - void checkOnePair(size_t i, ref EscapeBy eb, ref EscapeBy eb2, + //printf("escapeBy()\n"); + //foreach (const i, ref eb; escapeBy[0 .. $]) printf("i: %d eb: %s\n", cast(int)i, eb.exp.toChars()); + + void checkOnePair(size_t i, size_t j, ref EscapeBy eb, ref EscapeBy eb2, VarDeclaration v, VarDeclaration v2, bool of) { - if (log) printf("v2: `%s`\n", v2.toChars()); + //printf("i: %d j: %d\n", cast(int)i, cast(int)j); + if (log) printf("v: `%s` v2: `%s`\n", v.toChars(), v2.toChars()); if (v2 != v) return; //printf("v %d v2 %d\n", eb.isMutable, eb2.isMutable); @@ -174,19 +186,33 @@ bool checkMutableArguments(Scope* sc, FuncDeclaration fd, TypeFunction tf, if (!tf.islive && !(sc.useDIP1000 == FeatureState.enabled && sc.func && sc.func.setUnsafe())) return; + auto arg1 = escapeBy[i].exp; + auto arg2 = escapeBy[j].exp; + + if (log) + { + printf("arg1: %s arg2: %s\n", arg1.toChars(), arg2.toChars()); + printAST(arg1); + printAST(arg2); + } + if (arg1 && arg2 && !expOverlap(arg1, arg2)) // sure they don't overlap + return; + if (!gag) { + const s2 = arg2 ? arg2.toChars() : v2.toChars(); // int i; funcThatEscapes(ref int i); // funcThatEscapes(i); // error escaping reference _to_ `i` // int* j; funcThatEscapes2(int* j); // funcThatEscapes2(j); // error escaping reference _of_ `i` const(char)* referenceVerb = of ? "of" : "to"; const(char)* msg = eb.isMutable && eb2.isMutable - ? "more than one mutable reference %s `%s` in arguments to `%s()`" - : "mutable and const references %s `%s` in arguments to `%s()`"; - sc.eSink.error((*arguments)[i].loc, msg, + ? "more than one mutable reference %s `%s` in arguments `%s` and `%s` to `%s()`" + : "mutable and const references %s `%s` in arguments `%s` and `%s` to `%s()`"; + sc.eSink.error(escapeBy[i].loc, msg, referenceVerb, v.toChars(), + arg1.toChars(), s2, fd ? fd.toPrettyChars() : "indirectly"); } errors = true; @@ -203,11 +229,11 @@ bool checkMutableArguments(Scope* sc, FuncDeclaration fd, TypeFunction tf, } if (byval && !v.type.hasPointers()) continue; - foreach (ref eb2; escapeBy[i + 1 .. $]) + foreach (j, ref eb2; escapeBy[i + 1 .. $]) { foreach (VarDeclaration v2; byval ? eb2.er.byvalue : eb2.er.byref) { - checkOnePair(i, eb, eb2, v, v2, byval); + checkOnePair(i, i + 1 + j, eb, eb2, v, v2, byval); } } } @@ -2650,3 +2676,50 @@ private bool isTypesafeVariadicArray(VarDeclaration v) } return false; } + +/********************************* + * See if we can prove that e1 and e2 do not overlap. + * Params: + * e1 = first expression + * e2 = second expression + * Returns: + * false if they definitely do not overlap + */ +private bool expOverlap(Expression e1, Expression e2) +{ + //printf("expOverlap() %s %s\n", e1.toChars(), e2.toChars()); + + const sz1 = e1.type.size(); + const sz2 = e2.type.size(); + assert(sz1 != SIZE_INVALID && sz2 != SIZE_INVALID); + uint offset1 = 0; + uint offset2 = 0; + while (1) + { + auto dve1 = e1.isDotVarExp(); + auto dve2 = e2.isDotVarExp(); + if (dve1 && dve2) + { + auto field1 = dve1.var.isVarDeclaration(); + auto field2 = dve2.var.isVarDeclaration(); + if (field1 && field2) + { + offset1 += field1.offset; + offset2 += field2.offset; + e1 = dve1.e1; + e2 = dve2.e1; + } + else + break; + } + else + break; + } + //printf("%d.%d %d.%d\n", cast(int)offset1, cast(int)sz1, cast(int)offset2, cast(int)sz2); + if (offset1 + sz1 <= offset2 || + offset2 + sz2 <= offset1) + { + return false; + } + return true; +} diff --git a/compiler/test/fail_compilation/fob1.d b/compiler/test/fail_compilation/fob1.d index d11a7a69916e..ecf867ad85e4 100644 --- a/compiler/test/fail_compilation/fob1.d +++ b/compiler/test/fail_compilation/fob1.d @@ -65,7 +65,7 @@ fail_compilation/fob1.d(405): Error: variable `fob1.foo4.bq` has undefined state /* TEST_OUTPUT: --- -fail_compilation/fob1.d(503): Error: more than one mutable reference to `a` in arguments to `fob1.foo5()` +fail_compilation/fob1.d(503): Error: more than one mutable reference to `a` in arguments `a` and `a` to `fob1.foo5()` --- */ diff --git a/compiler/test/fail_compilation/fob2.d b/compiler/test/fail_compilation/fob2.d index e9179da74057..c333606e707a 100644 --- a/compiler/test/fail_compilation/fob2.d +++ b/compiler/test/fail_compilation/fob2.d @@ -1,5 +1,6 @@ /* Testing Ownership/Borrowing system REQUIRED_ARGS: -preview=dip1021 +DISABLED: win32 linux32 osx32 freebsd32 */ int* malloc(); @@ -28,7 +29,7 @@ fail_compilation/fob2.d(103): Error: variable `fob2.foo1.p` is not disposed of b /* TEST_OUTPUT: --- -fail_compilation/fob2.d(203): Error: more than one mutable reference of `p` in arguments to `fob2.foo2()` +fail_compilation/fob2.d(203): Error: more than one mutable reference of `p` in arguments `p` and `p + 4L` to `fob2.foo2()` --- */ @@ -143,7 +144,7 @@ fail_compilation/fob2.d(515): Error: variable `fob2.test52.p` has undefined stat /* TEST_OUTPUT: --- fail_compilation/fob2.d(603): Error: variable `fob2.test6.p` is not disposed of before return -fail_compilation/fob2.d(612): Error: more than one mutable reference of `p` in arguments to `fob2.foo6b()` +fail_compilation/fob2.d(612): Error: more than one mutable reference of `p` in arguments `p` and `p` to `fob2.foo6b()` --- */ diff --git a/compiler/test/fail_compilation/test1021.d b/compiler/test/fail_compilation/test1021.d index b3bb5b3ef2cd..172565a39d72 100644 --- a/compiler/test/fail_compilation/test1021.d +++ b/compiler/test/fail_compilation/test1021.d @@ -5,10 +5,10 @@ /* TEST_OUTPUT: --- -fail_compilation/test1021.d(1009): Error: more than one mutable reference of `p` in arguments to `test1021.fooa()` -fail_compilation/test1021.d(1010): Error: mutable and const references of `p` in arguments to `test1021.foob()` -fail_compilation/test1021.d(1011): Error: mutable and const references of `p` in arguments to `test1021.fooc()` -fail_compilation/test1021.d(1013): Error: more than one mutable reference of `p` in arguments to `test1021.fooe()` +fail_compilation/test1021.d(1009): Error: more than one mutable reference of `p` in arguments `p` and `p` to `test1021.fooa()` +fail_compilation/test1021.d(1010): Error: mutable and const references of `p` in arguments `cast(const(int)*)p` and `p` to `test1021.foob()` +fail_compilation/test1021.d(1011): Error: mutable and const references of `p` in arguments `p` and `cast(const(int)*)p` to `test1021.fooc()` +fail_compilation/test1021.d(1013): Error: more than one mutable reference of `p` in arguments `p` and `p` to `test1021.fooe()` --- */ @@ -33,10 +33,10 @@ void test1(int* p) /* TEST_OUTPUT: --- -fail_compilation/test1021.d(2010): Error: more than one mutable reference to `i` in arguments to `test1021.fopa()` -fail_compilation/test1021.d(2011): Error: mutable and const references to `i` in arguments to `test1021.fopb()` -fail_compilation/test1021.d(2012): Error: mutable and const references to `i` in arguments to `test1021.fopc()` -fail_compilation/test1021.d(2014): Error: more than one mutable reference to `i` in arguments to `test1021.fope()` +fail_compilation/test1021.d(2010): Error: more than one mutable reference to `i` in arguments `i` and `toPtr(i)` to `test1021.fopa()` +fail_compilation/test1021.d(2011): Error: mutable and const references to `i` in arguments `i` and `toPtr(i)` to `test1021.fopb()` +fail_compilation/test1021.d(2012): Error: mutable and const references to `i` in arguments `i` and `toPtr(i)` to `test1021.fopc()` +fail_compilation/test1021.d(2014): Error: more than one mutable reference to `i` in arguments `i` and `toPtr(i)` to `test1021.fope()` --- */ @@ -62,8 +62,8 @@ void test2() /* TEST_OUTPUT: --- -fail_compilation/test1021.d(3015): Error: more than one mutable reference to `s` in arguments to `test1021.S.method()` -fail_compilation/test1021.d(3019): Error: more than one mutable reference of `c` in arguments to `test1021.C.method()` +fail_compilation/test1021.d(3015): Error: more than one mutable reference to `s` in arguments `s` and `s` to `test1021.S.method()` +fail_compilation/test1021.d(3019): Error: more than one mutable reference of `c` in arguments `c` and `c` to `test1021.C.method()` --- */ @@ -94,7 +94,7 @@ void test3() /* TEST_OUTPUT: --- -fail_compilation/test1021.d(4008): Error: more than one mutable reference to `i` in arguments to `test1021.test4.nested()` +fail_compilation/test1021.d(4008): Error: more than one mutable reference to `i` in arguments `i` and `i` to `test1021.test4.nested()` --- */ @@ -115,7 +115,7 @@ void test4() /* TEST_OUTPUT: --- -fail_compilation/test1021.d(5012): Error: more than one mutable reference of `s` in arguments to `test1021.foo5()` +fail_compilation/test1021.d(5012): Error: more than one mutable reference of `s` in arguments `s` and `s` to `test1021.foo5()` --- */ @@ -167,3 +167,26 @@ void test5c() S5c s; foo5(s, s); } + +/*********************************************/ + +/* TEST_OUTPUT: +--- +fail_compilation/test1021.d(6011): Error: more than one mutable reference to `s` in arguments `s.a.g` and `s.a.g` to `test1021.blender()` +--- +*/ + +#line 6000 + +struct D { int f, g; } +struct S6 { D a, b; } + +void blender(out int x, ref int y); + +void mixer() +{ + S6 s; + blender(s.a.f, s.b.f); + blender(s.a.f, s.b.g); + blender(s.a.g, s.a.g); +} diff --git a/druntime/Makefile b/druntime/Makefile index 5aac2b73ef81..cab532df6224 100644 --- a/druntime/Makefile +++ b/druntime/Makefile @@ -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=dip1021 -preview=fieldwise $(MODEL_FLAG) $(PIC) $(OPTIONAL_COVERAGE) -preview=dtorfields ifeq ($(BUILD),debug) UDFLAGS += -g -debug DFLAGS:=$(UDFLAGS) diff --git a/druntime/src/core/demangle.d b/druntime/src/core/demangle.d index 272ee1e1ba4b..66fc57f49fa4 100644 --- a/druntime/src/core/demangle.d +++ b/druntime/src/core/demangle.d @@ -752,7 +752,9 @@ pure @safe: static if (__traits(hasMember, Hooks, "parseType")) { - auto n = hooks.parseType(errStatus, this, null); + // passing two mutable references to parseType(), not safe, needs redesign + auto n = this.hooks.parseType(errStatus, this, null); + if (errStatus) return dst.bslice_empty; else @@ -2250,7 +2252,7 @@ char[] demangleType( const(char)[] buf, char[] dst = null ) nothrow pure @safe /** * reencode a mangled symbol name that might include duplicate occurrences -* of the same identifier by replacing all but the first occurence with +* of the same identifier by replacing all but the first occurrence with * a back reference. * * Params: @@ -2379,6 +2381,7 @@ char[] reencodeMangled(return scope const(char)[] mangled) nothrow pure @safe return true; } + @trusted char[] parseType( out bool errStatus, ref Remangle d, char[] name ) return scope nothrow { if (d.front != 'Q') @@ -2386,7 +2389,7 @@ char[] reencodeMangled(return scope const(char)[] mangled) nothrow pure @safe flushPosition(d); - auto refPos = d.pos; + const refPos = d.pos; d.popFront(); auto n = d.decodeBackref(); if (n == 0 || n > refPos) diff --git a/druntime/src/core/internal/postblit.d b/druntime/src/core/internal/postblit.d index ed4ec1b7ff4a..8c95037012a4 100644 --- a/druntime/src/core/internal/postblit.d +++ b/druntime/src/core/internal/postblit.d @@ -151,7 +151,8 @@ package void postblitRecurse(E, size_t n)(ref E[n] arr) // Test handling of fixed-length arrays // Separate from first test because of https://issues.dlang.org/show_bug.cgi?id=14242 -@safe unittest +//@safe fails dip1021 +unittest { string[] order; diff --git a/druntime/src/core/lifetime.d b/druntime/src/core/lifetime.d index 9e563ad43a30..56474501613a 100644 --- a/druntime/src/core/lifetime.d +++ b/druntime/src/core/lifetime.d @@ -1955,7 +1955,8 @@ pure nothrow @safe @nogc unittest } alias T = void function() @system nothrow; - static assert(is(typeof({ S s; move(s); }) == T)); +// fails because of DIP1021, see below +// static assert(is(typeof({ S s; move(s); }) == T)); static assert(is(typeof({ S s; move(s, s); }) == T)); } @@ -1978,7 +1979,9 @@ private T moveImpl(T)(return scope ref T source) { // Properly infer safety from moveEmplaceImpl as the implementation below // might void-initialize pointers in result and hence needs to be @trusted - if (false) moveEmplaceImpl(source, source); + +// not compatible with DIP1021, passing 2 mutable references to same data +// if (false) moveEmplaceImpl(source, source); return trustedMoveImpl(source); } diff --git a/druntime/src/object.d b/druntime/src/object.d index 4264ecf6fd62..b52031471da3 100644 --- a/druntime/src/object.d +++ b/druntime/src/object.d @@ -373,9 +373,10 @@ if ((is(LHS : const Object) || is(LHS : const shared Object)) && { this(int flag) { super(flag); } - bool opEquals(const Base o) @safe + bool opEquals(const Base o) @trusted { - fEquals++; + fEquals++; // not @safe with dip1021 because another mutable ref this + // is passed to access fEquals return flag == o.flag; } } @@ -384,7 +385,7 @@ if ((is(LHS : const Object) || is(LHS : const shared Object)) && { this(int flag) { super(flag); } - bool opEquals(const Base o) @safe + bool opEquals(const Base o) @trusted { gEquals++; return flag == o.flag; @@ -4717,6 +4718,7 @@ public import core.lifetime : _d_newitemT; public @trusted @nogc nothrow pure extern (C) void _d_delThrowable(scope Throwable); // Compare class and interface objects for ordering. +@trusted // because the arguments should be const int __cmp(C1, C2)(C1 lhs, C2 rhs) if ((is(C1 : const(Object)) || (is(C1 == interface) && (__traits(getLinkage, C1) == "D"))) && (is(C2 : const(Object)) || (is(C2 == interface) && (__traits(getLinkage, C2) == "D")))) diff --git a/druntime/test/common.mak b/druntime/test/common.mak index 50b66acbb351..0938c3074a03 100644 --- a/druntime/test/common.mak +++ b/druntime/test/common.mak @@ -27,7 +27,7 @@ CFLAGS_BASE:=$(if $(findstring $(OS),windows),/Wall,$(MODEL_FLAG) $(PIC) -Wall) ifeq (osx64,$(OS)$(MODEL)) CFLAGS_BASE+=--target=x86_64-darwin-apple # ARM cpu is not supported by dmd endif -DFLAGS:=$(MODEL_FLAG) $(PIC) -w -I../../src -I../../import -I$(SRC) -defaultlib= -preview=dip1000 $(if $(findstring $(OS),windows),,-L-lpthread -L-lm $(LINKDL)) +DFLAGS:=$(MODEL_FLAG) $(PIC) -w -I../../src -I../../import -I$(SRC) -defaultlib= -preview=dip1000 -preview=dip1021 $(if $(findstring $(OS),windows),,-L-lpthread -L-lm $(LINKDL)) # LINK_SHARED may be set by importing makefile DFLAGS+=$(if $(LINK_SHARED),-L$(DRUNTIME_IMPLIB) $(if $(findstring $(OS),windows),-dllimport=all),-L$(DRUNTIME)) ifeq ($(BUILD),debug)