Skip to content

Commit

Permalink
Make in a first-class storage class, not a const [scope] alias
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Geod24 committed Jul 30, 2020
1 parent 3495c57 commit d4b0800
Show file tree
Hide file tree
Showing 10 changed files with 144 additions and 21 deletions.
8 changes: 7 additions & 1 deletion src/dmd/dmangle.d
Expand Up @@ -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
Expand All @@ -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 : '@',
Expand Down Expand Up @@ -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');
Expand Down
5 changes: 5 additions & 0 deletions src/dmd/dsymbolsem.d
Expand Up @@ -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);
Expand Down
18 changes: 10 additions & 8 deletions src/dmd/hdrgen.d
Expand Up @@ -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_;
Expand All @@ -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)
Expand Down Expand Up @@ -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(' ');
Expand Down
24 changes: 20 additions & 4 deletions src/dmd/mtype.d
Expand Up @@ -4427,6 +4427,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;

Expand Down Expand Up @@ -6823,12 +6827,24 @@ extern (C++) final class Parameter : ASTNode
* 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
Expand Down
5 changes: 1 addition & 4 deletions src/dmd/typesem.d
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
}
Expand Down
2 changes: 1 addition & 1 deletion test/compilable/previewin.d
Expand Up @@ -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*));
28 changes: 28 additions & 0 deletions 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) {}
4 changes: 2 additions & 2 deletions 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`
---
*/

Expand Down
69 changes: 69 additions & 0 deletions test/fail_compilation/previewin.d
@@ -0,0 +1,69 @@
/*
REQUIRED_ARGS: -preview=in -preview=dip1000
TEST_OUTPUT:
---
fail_compilation/previewin.d(3): Error: function `previewin.func1(void function(ulong[8]) dg)` is not callable using argument types `(void function(in ulong[8]))`
fail_compilation/previewin.d(3): cannot pass argument `& func_byRef` of type `void function(in ulong[8])` to parameter `void function(ulong[8]) dg`
fail_compilation/previewin.d(4): Error: function `previewin.func2(void function(ref ulong[8]) dg)` is not callable using argument types `(void function(in ulong[8]))`
fail_compilation/previewin.d(4): cannot pass argument `& func_byRef` of type `void function(in ulong[8])` to parameter `void function(ref ulong[8]) dg`
fail_compilation/previewin.d(5): Error: function `previewin.func3(void function(ref const(ulong[8])) dg)` is not callable using argument types `(void function(in ulong[8]))`
fail_compilation/previewin.d(5): cannot pass argument `& func_byRef` of type `void function(in ulong[8])` to parameter `void function(ref const(ulong[8])) dg`
fail_compilation/previewin.d(7): Error: function `previewin.func4(void function(ref ulong) dg)` is not callable using argument types `(void function(in ulong))`
fail_compilation/previewin.d(7): cannot pass argument `& func_byValue` of type `void function(in ulong)` to parameter `void function(ref ulong) dg`
fail_compilation/previewin.d(41): Error: scope variable `arg` assigned to non-scope `myGlobal`
fail_compilation/previewin.d(42): Error: scope variable `arg` assigned to non-scope `myGlobal`
fail_compilation/previewin.d(43): Error: scope variable `arg` may not be returned
fail_compilation/previewin.d(44): Error: scope variable `arg` assigned to `escape` with longer lifetime
fail_compilation/previewin.d(48): Error: returning `arg` escapes a reference to parameter `arg`, perhaps annotate with `return`
---
*/

#line 1
void main ()
{
func1(&func_byRef); // No
func2(&func_byRef); // No
func3(&func_byRef); // Yes

func4(&func_byValue); // No
func5(&func_byValue); // Yes

func6(&func_byValue2); // Yes
func7(&func_byValue3); // Yes

tryEscape("Hello World"); // Yes by `tryEscape` is NG
}

// Takes by `scope ref const`
void func_byRef(in ulong[8]) {}
// Takes by `scope const`
void func_byValue(in size_t) {}

// Error: `ulong[8]` is passed by `ref`
void func1(void function(scope ulong[8]) dg) {}
// Error: Missing `scope` on a `ref`
void func2(void function(ref ulong[8]) dg) {}
// Works: `scope ref`
void func3(void function(scope const ref ulong[8]) dg) {}

// Error: `size_t` is passed by value
void func4(void function(ref size_t) dg) {}
// Works: By value `scope const`
void func5(void function(scope const size_t) dg) {}

// This works for arrays:
void func_byValue2(in char[]) {}
void func6(void function(char[]) dg) {}
void func_byValue3(scope const(char)[]) {}
void func7(void function(in char[]) dg) {}

// Make sure things cannot be escaped (`scope` is applied)
const(char)[] myGlobal;
void tryEscape(in char[] arg) @safe { myGlobal = arg; }
void tryEscape2(scope const char[] arg) @safe { myGlobal = arg; }
const(char)[] tryEscape3(in char[] arg) @safe { return arg; }
void tryEscape4(in char[] arg, ref const(char)[] escape) @safe { escape = arg; }
// Okay: value type
ulong[8] tryEscape5(in ulong[8] arg) @safe { return arg; }
// NG: Ref
ref const(ulong[8]) tryEscape6(in ulong[8] arg) @safe { return arg; }
2 changes: 1 addition & 1 deletion test/runnable/xtest46.d
Expand Up @@ -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)");
}

Expand Down

0 comments on commit d4b0800

Please sign in to comment.