From 91be7f4e126c89c944f990af310caede5fdc969a Mon Sep 17 00:00:00 2001 From: Geod24 Date: Mon, 27 Jul 2020 18:12:46 +0900 Subject: [PATCH] Make `in` a first-class storage class, not a `const [scope]` alias For a long time, the 'in' storage class was only a second class citizen, merely an alias for 'const', which is a type constructor. While it was also documented for a long time as being 'scope', this was removed when 'scope' actually started to have an effect (when DIP1000 started to be implemented). Currently, a switch (-preview=in) allows to get back this 'scope'. However, the 'in' storage class does not really exists, it gets lowered to 'const [scope]' at an early stage by the compiler, which means that we expose what is essentially an implementation detail to the user. This led to a variety of problems for both developers and users. The most obvious one was that functions involving `in` would show up simply as `const` (or `const scope`), both in generated header and error messagges, hindering the ability to do header generation with `-preview=in` as the same value for this switch was needed for both producer and consumer. Another issue was that the result of `__traits(getParameterStorageClass)` was inaccurate, giving either an empty tuple or `scope`. Lastly, the lack of mangling for `in` means that stack trace would show the wrong signature for a function, potentially confusing the users. For compiler developers, the `in` storage class simply couldn't be relied on, as it was replaced by `const [scope]` rather early in the semantic phase (in TypeFunction's semantic), which lead to some dead code (e.g. in dmangle). After this change, the class will show up properly in error message, and in `getParameterStorageClass`, which can be a surprising change for users. However, it is not expected that code can break as a result, unless they break as a result of using the `-preview=in` switch. --- src/dmd/dmangle.d | 8 +++++++- src/dmd/dsymbolsem.d | 5 +++++ src/dmd/hdrgen.d | 18 ++++++++++-------- src/dmd/mtype.d | 28 ++++++++++++++++++++++++---- src/dmd/typesem.d | 5 +---- test/compilable/previewin.d | 2 +- test/fail_compilation/diagin.d | 28 ++++++++++++++++++++++++++++ test/fail_compilation/ice10922.d | 4 ++-- test/runnable/xtest46.d | 2 +- 9 files changed, 79 insertions(+), 21 deletions(-) create mode 100644 test/fail_compilation/diagin.d diff --git a/src/dmd/dmangle.d b/src/dmd/dmangle.d index c20fd2aeaf4f..2ae4c5116c75 100644 --- a/src/dmd/dmangle.d +++ b/src/dmd/dmangle.d @@ -79,7 +79,7 @@ private immutable char[TMAX] mangleChar = Tfunction : 'F', // D function Tsarray : 'G', Taarray : 'H', - Tident : 'I', + // I // in // J // out // K // ref // L // lazy @@ -99,6 +99,7 @@ private immutable char[TMAX] mangleChar = // Z // not variadic, end of parameters // '@' shouldn't appear anywhere in the deco'd names + Tident : '@', Tinstance : '@', Terror : '@', Ttypeof : '@', @@ -1112,7 +1113,12 @@ public: switch (p.storageClass & (STC.IOR | STC.lazy_)) { case 0: + break; case STC.in_: + buf.writeByte('I'); + break; + case STC.in_ | STC.ref_: + buf.writestring("IK"); break; case STC.out_: buf.writeByte('J'); diff --git a/src/dmd/dsymbolsem.d b/src/dmd/dsymbolsem.d index e0ebac60f031..5a7e01580bc9 100644 --- a/src/dmd/dsymbolsem.d +++ b/src/dmd/dsymbolsem.d @@ -1102,6 +1102,11 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor dsym.storage_class &= ~stc; // strip off } + // At this point we can add `scope` to the STC instead of `in`, + // because we are never going to use this variable's STC for user messages + if (dsym.storage_class & STC.in_ && global.params.previewIn) + dsym.storage_class |= STC.scope_; + if (dsym.storage_class & STC.scope_) { StorageClass stc = dsym.storage_class & (STC.static_ | STC.extern_ | STC.manifest | STC.tls | STC.gshared); diff --git a/src/dmd/hdrgen.d b/src/dmd/hdrgen.d index 795a47a5c89f..63af7d5b3de3 100644 --- a/src/dmd/hdrgen.d +++ b/src/dmd/hdrgen.d @@ -3057,17 +3057,18 @@ private void parameterToBuffer(Parameter p, OutBuffer* buf, HdrGenState* hgs) if (p.storageClass & STC.return_) buf.writestring("return "); - if (p.storageClass & STC.out_) - buf.writestring("out "); - else if (p.storageClass & STC.ref_) - buf.writestring("ref "); - else if (p.storageClass & STC.in_) + if (p.storageClass & STC.in_) buf.writestring("in "); + else if (p.storageClass & STC.out_) + buf.writestring("out "); else if (p.storageClass & STC.lazy_) buf.writestring("lazy "); else if (p.storageClass & STC.alias_) buf.writestring("alias "); + if (p.storageClass & STC.ref_) + buf.writestring("ref "); + StorageClass stc = p.storageClass; if (p.type && p.type.mod & MODFlags.shared_) stc &= ~STC.shared_; @@ -3089,7 +3090,7 @@ private void parameterToBuffer(Parameter p, OutBuffer* buf, HdrGenState* hgs) } else { - typeToBuffer(p.type, p.ident, buf, hgs); + typeToBuffer(p.type, p.ident, buf, hgs, (stc & STC.in_) ? MODFlags.const_ : 0); } if (p.defaultArg) @@ -3220,14 +3221,15 @@ private void expToBuffer(Expression e, PREC pr, OutBuffer* buf, HdrGenState* hgs /************************************************** * An entry point to pretty-print type. */ -private void typeToBuffer(Type t, const Identifier ident, OutBuffer* buf, HdrGenState* hgs) +private void typeToBuffer(Type t, const Identifier ident, OutBuffer* buf, HdrGenState* hgs, + ubyte modMask = 0) { if (auto tf = t.isTypeFunction()) { visitFuncIdentWithPrefix(tf, ident, null, buf, hgs); return; } - visitWithMask(t, 0, buf, hgs); + visitWithMask(t, modMask, buf, hgs); if (ident) { buf.writeByte(' '); diff --git a/src/dmd/mtype.d b/src/dmd/mtype.d index 207172d98bd6..9e515765734e 100644 --- a/src/dmd/mtype.d +++ b/src/dmd/mtype.d @@ -4440,6 +4440,10 @@ extern (C++) final class TypeFunction : TypeNext if (!global.params.vsafe) return stc; + // When the preview switch is enable, `in` parameters are `scope` + if (stc & STC.in_ && global.params.previewIn) + return stc | STC.scope_; + if (stc & (STC.scope_ | STC.return_ | STC.lazy_) || purity == PURE.impure) return stc; @@ -6829,19 +6833,35 @@ extern (C++) final class Parameter : ASTNode /********************************* * Compute covariance of parameters `this` and `p` * as determined by the storage classes of both. + * * Params: * returnByRef = true if the function returns by ref * p = Parameter to compare with + * previewIn = Whether `-previewIn` is being used, and thus if + * `in` means `scope`. + * * Returns: * true = `this` can be used in place of `p` * false = nope */ - bool isCovariant(bool returnByRef, const Parameter p) const pure nothrow @nogc @safe + bool isCovariant(bool returnByRef, const Parameter p, bool previewIn = global.params.previewIn) + const pure nothrow @nogc @safe { - enum stc = STC.IOR | STC.lazy_; - if ((this.storageClass & stc) != (p.storageClass & stc)) + ulong thisSTC = this.storageClass; + ulong otherSTC = p.storageClass; + + if (previewIn) + { + if (thisSTC & STC.in_) + thisSTC |= STC.scope_; + if (otherSTC & STC.in_) + otherSTC |= STC.scope_; + } + + enum stc = STC.ref_ | STC.out_ | STC.lazy_; + if ((thisSTC & stc) != (otherSTC & stc)) return false; - return isCovariantScope(returnByRef, this.storageClass, p.storageClass); + return isCovariantScope(returnByRef, thisSTC, otherSTC); } extern (D) private static bool isCovariantScope(bool returnByRef, StorageClass from, StorageClass to) pure nothrow @nogc @safe diff --git a/src/dmd/typesem.d b/src/dmd/typesem.d index 0bb11decf6a4..38095e0df2dd 100644 --- a/src/dmd/typesem.d +++ b/src/dmd/typesem.d @@ -1261,9 +1261,6 @@ extern(C++) Type typeSemantic(Type t, const ref Loc loc, Scope* sc) for (size_t i = 0; i < dim; i++) { Parameter fparam = tf.parameterList[i]; - // If `-preview=in` is on, set `scope` as well - if ((fparam.storageClass & STC.in_) && global.params.previewIn) - fparam.storageClass |= STC.scope_; fparam.storageClass |= STC.parameter; mtype.inuse++; fparam.type = fparam.type.typeSemantic(loc, argsc); @@ -1520,7 +1517,7 @@ extern(C++) Type typeSemantic(Type t, const ref Loc loc, Scope* sc) } // Remove redundant storage classes for type, they are already applied - fparam.storageClass &= ~(STC.TYPECTOR | STC.in_); + fparam.storageClass &= ~(STC.TYPECTOR); } argsc.pop(); } diff --git a/test/compilable/previewin.d b/test/compilable/previewin.d index d10e14b08969..d096ba9f17d9 100644 --- a/test/compilable/previewin.d +++ b/test/compilable/previewin.d @@ -3,5 +3,5 @@ @safe: void fun(in int* inParam); -static assert(__traits(getParameterStorageClasses, fun, 0)[0] == "scope"); +static assert([__traits(getParameterStorageClasses, fun, 0)] == ["in"]); static assert (is(typeof(fun) P == __parameters) && is(P[0] == const int*)); diff --git a/test/fail_compilation/diagin.d b/test/fail_compilation/diagin.d new file mode 100644 index 000000000000..61f65db953f1 --- /dev/null +++ b/test/fail_compilation/diagin.d @@ -0,0 +1,28 @@ +/* +PERMUTE_ARGS: -preview=in +TEST_OUTPUT: +--- +fail_compilation/diagin.d(18): Error: function `diagin.foo(in string)` is not callable using argument types `()` +fail_compilation/diagin.d(18): missing argument for parameter #1: `in string` +fail_compilation/diagin.d(19): Error: function `diagin.foo1(in ref string)` is not callable using argument types `()` +fail_compilation/diagin.d(19): missing argument for parameter #1: `in ref string` +fail_compilation/diagin.d(20): Error: template `diagin.foo2` cannot deduce function from argument types `!()(int)`, candidates are: +fail_compilation/diagin.d(27): `foo2(T)(in T v, string)` +fail_compilation/diagin.d(22): Error: template `diagin.foo3` cannot deduce function from argument types `!()(bool[])`, candidates are: +fail_compilation/diagin.d(28): `foo3(T)(in ref T v, string)` +--- + */ + +void main () +{ + foo(); + foo1(); + foo2(42); + bool[] lvalue; + foo3(lvalue); +} + +void foo(in string) {} +void foo1(in ref string) {} +void foo2(T)(in T v, string) {} +void foo3(T)(ref in T v, string) {} diff --git a/test/fail_compilation/ice10922.d b/test/fail_compilation/ice10922.d index 9eeb622300a4..c227ee55eb4f 100644 --- a/test/fail_compilation/ice10922.d +++ b/test/fail_compilation/ice10922.d @@ -1,8 +1,8 @@ /* TEST_OUTPUT: --- -fail_compilation/ice10922.d(10): Error: function `ice10922.__lambda4(const(uint) n)` is not callable using argument types `()` -fail_compilation/ice10922.d(10): missing argument for parameter #1: `const(uint) n` +fail_compilation/ice10922.d(10): Error: function `ice10922.__lambda4(in uint n)` is not callable using argument types `()` +fail_compilation/ice10922.d(10): missing argument for parameter #1: `in uint n` --- */ diff --git a/test/runnable/xtest46.d b/test/runnable/xtest46.d index fdd21d39d88e..b77d83f4c9fa 100644 --- a/test/runnable/xtest46.d +++ b/test/runnable/xtest46.d @@ -4963,7 +4963,7 @@ void test6763() static assert(typeof(f6763).stringof == "void(int _param_0)"); static assert(typeof(c6763).stringof == "void(const(int) _param_0)"); static assert(typeof(r6763).stringof == "void(ref int _param_0)"); - static assert(typeof(i6763).stringof == "void(const(int) _param_0)"); + static assert(typeof(i6763).stringof == "void(in int _param_0)"); static assert(typeof(o6763).stringof == "void(out int _param_0)"); }