Skip to content

do not emit mangling for inferred 'return' attribute #9476

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dlang-bot merged 1 commit into from
Mar 22, 2019
Merged

do not emit mangling for inferred 'return' attribute #9476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions src/dmd/astbase.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ struct ASTBase
scopeinferred = (1L << 49), // 'scope' has been inferred and should not be part of mangling
future = (1L << 50), // introducing new base class function
local = (1L << 51), // do not forward (see dmd.dsymbol.ForwardingScopeDsymbol).
returninferred = (1L << 52), // 'return' has been inferred and should not be part of mangling
Copy link
Member

@ibuclaw ibuclaw Mar 22, 2019

Choose a reason for hiding this comment

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

Wouldn't it be better to have just one STCinferred?

Copy link
Member Author

@WalterBright WalterBright Mar 22, 2019

Choose a reason for hiding this comment

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

Either return or scope or both can be present or inferred.

Copy link
Member

@ibuclaw ibuclaw Mar 22, 2019

Choose a reason for hiding this comment

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

I'd not have expected inference to happen if either is explicitly given, but Ok.

Some of the bit flags may require a little pruning as there aren't many available left now.


TYPECTOR = (STC.const_ | STC.immutable_ | STC.shared_ | STC.wild),
FUNCATTR = (STC.ref_ | STC.nothrow_ | STC.nogc | STC.pure_ | STC.property | STC.safe | STC.trusted | STC.system),
Expand Down
1 change: 1 addition & 0 deletions src/dmd/declaration.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ enum STC : long
scopeinferred = (1L << 49), // 'scope' has been inferred and should not be part of mangling
future = (1L << 50), // introducing new base class function
local = (1L << 51), // do not forward (see dmd.dsymbol.ForwardingScopeDsymbol).
returninferred = (1L << 52), // 'return' has been inferred and should not be part of mangling

TYPECTOR = (STC.const_ | STC.immutable_ | STC.shared_ | STC.wild),
FUNCATTR = (STC.ref_ | STC.nothrow_ | STC.nogc | STC.pure_ | STC.property | STC.safe | STC.trusted | STC.system),
Expand Down
1 change: 1 addition & 0 deletions src/dmd/declaration.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ struct IntRange;
#define STCscopeinferred 0x2000000000000LL // 'scope' has been inferred and should not be part of mangling
#define STCfuture 0x4000000000000LL // introducing new base class function
#define STClocal 0x8000000000000LL // do not forward (see dmd.dsymbol.ForwardingScopeDsymbol).
#define STCreturninferred 0x10000000000000LL // 'return' has been inferred and should not be part of mangling

void ObjectNotFound(Identifier *id);

Expand Down
3 changes: 2 additions & 1 deletion src/dmd/dmangle.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1066,7 +1066,8 @@ public:
if (p.storageClass & STC.scope_ && !(p.storageClass & STC.scopeinferred))
buf.writeByte('M');
// 'return inout ref' is the same as 'inout ref'
if ((p.storageClass & (STC.return_ | STC.wild)) == STC.return_)
if ((p.storageClass & (STC.return_ | STC.wild)) == STC.return_ &&
!(p.storageClass & STC.returninferred))
buf.writestring("Nk");
switch (p.storageClass & (STC.in_ | STC.out_ | STC.ref_ | STC.lazy_))
{
Expand Down
13 changes: 7 additions & 6 deletions src/dmd/escape.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,8 @@ bool checkAssignEscape(Scope* sc, Expression e, bool gag)
if (!va.isScope() && inferScope)
{ //printf("inferring scope for %s\n", va.toChars());
va.storage_class |= STC.scope_ | STC.scopeinferred;
va.storage_class |= v.storage_class & STC.return_;
if (v.storage_class & STC.return_)
va.storage_class |= STC.return_ | STC.returninferred;
Copy link
Member

@rainers rainers Mar 22, 2019

Choose a reason for hiding this comment

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

Bad indentation.

Doesn't this need a check for pre-existing return annotation?

if (!(va.storage_class & STC.return_))
    va.storage_class |= STC.return_ | STC.returninferred;

If scope and return can be inferred independently, the if-condition !va.isScope() seems to disallow that.

}
continue;
}
Expand Down Expand Up @@ -1071,7 +1072,7 @@ private bool checkReturnEscapeImpl(Scope* sc, Expression e, bool refs, bool gag)
* Because dg.ptr points to x, this is returning dt.ptr+offset
*/
if (global.params.vsafe)
sc.func.storage_class |= STC.return_;
sc.func.storage_class |= STC.return_ | STC.returninferred;
}

}
Expand Down Expand Up @@ -1141,14 +1142,14 @@ private void inferReturn(FuncDeclaration fd, VarDeclaration v)
// v is a local in the current function

//printf("for function '%s' inferring 'return' for variable '%s'\n", fd.toChars(), v.toChars());
v.storage_class |= STC.return_;
v.storage_class |= STC.return_ | STC.returninferred;

TypeFunction tf = cast(TypeFunction)fd.type;
if (v == fd.vthis)
{
/* v is the 'this' reference, so mark the function
*/
fd.storage_class |= STC.return_;
fd.storage_class |= STC.return_ | STC.returninferred;
if (tf.ty == Tfunction)
{
//printf("'this' too %p %s\n", tf, sc.func.toChars());
Expand All @@ -1166,7 +1167,7 @@ private void inferReturn(FuncDeclaration fd, VarDeclaration v)
Parameter p = tf.parameterList[i];
if (p.ident == v.ident)
{
p.storageClass |= STC.return_;
p.storageClass |= STC.return_ | STC.returninferred;
break; // there can be only one
}
}
Expand Down Expand Up @@ -1807,7 +1808,7 @@ void eliminateMaybeScopes(VarDeclaration[] array)
// v cannot be scope since it is assigned to a non-scope va
notMaybeScope(v);
if (!(v.storage_class & (STC.ref_ | STC.out_)))
v.storage_class &= ~STC.return_;
v.storage_class &= ~(STC.return_ | STC.returninferred);
changes = true;
}
}
Expand Down
12 changes: 12 additions & 0 deletions test/compilable/scopeinfer.d
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// PERMUTE_ARGS: -dip1000

// Mangling should be the same with or without inference of `return scope`

@safe:

auto foo(void* p) { return 0; }
static assert(typeof(foo).mangleof == "FNaNbNiNfPvZi");

auto bar(void* p) { return p; }
static assert(typeof(bar).mangleof == "FNaNbNiNfPvZQd");
Copy link
Member

@rainers rainers Mar 22, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Please also add tests that show explicit annotations make it to the mangling.

Copy link
Member Author

@WalterBright WalterBright Mar 22, 2019

Choose a reason for hiding this comment

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

see testscope2.d

Copy link
Member

@rainers rainers Mar 22, 2019

Choose a reason for hiding this comment

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

testscope2.d only runs with -dip25, but some of the code only affects -dip1000 AFAICT.

Copy link
Member

@rainers rainers Mar 22, 2019

Choose a reason for hiding this comment

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

BTW: you can also use core.demangle with CTFE for more readable asserts.