allow ref on locals, globals, and statics

[WalterBright]

trafficstars

Implementation of the proposal discussed in the n.g.:

https://www.digitalmars.com/d/archives/digitalmars/dip/development/First_Draft_ref_For_Variable_Declarations_74.html

[dlang-bot]

Thanks for your pull request, @WalterBright!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#16428"

[WalterBright]

are these such refs implicitly scope`?

Yes.

[thewilsonator]

Good to hear. Please add a fail_compilation (e.g. to diag9679.d) that tests that.

[WalterBright]

Please add a fail_compilation (e.g. to diag9679.d) that tests that.

done.

[WalterBright]

Cybershadow/DAutoTest gives no indication why it failed.

[thewilsonator]

As always , scroll to the bottom and work your way back up:

object.Exception@../tools/changed.d(256): Changelog entries should consist of one title line, a blank separator line, and a description.

[WalterBright]

@thewilsonator I did scroll through it, but did not see that message, lost in the thousands of lines of output. Thanks for finding it for me!

[WalterBright]

@ntrel it's the same as the way ref is used in any other context. Same semantics.

[WalterBright]

@ntrel it's the same behavior as:

void foo(ref int x);
void bar(ref int y)
{
    foo(y);
}

[WalterBright]

This is ready to go now.

[thewilsonator]

We should wait, the DIP has not even been submitted, let alone reviewed by the wider community.

[WalterBright]

the DIP has not even been submitted, let alone reviewed by the wider community

That's what https://www.digitalmars.com/d/archives/digitalmars/dip/development/First_Draft_ref_For_Variable_Declarations_74.html is for.

[JohanEngelen]

please don't merge yet

[JohanEngelen]

This requires a lot more tests, not just error diagnostic testing. A lot of things can go wrong in implementation, so please be extensive in testing. Certainly all examples from the DIP should be tested, and things like:

int global;

ref int foo() { return global; }

void main() {
    auto i = foo();
    i = 1;
    assert(global == 0);
}

[dkorpel]

Some things I found while testing:

ref can currently violate immutable by aliasing a mutable variable

void main()
{
    int x = 3;
    immutable ref int xr = x; // accepted!
    assert(xr == 3);
    x++;
    assert(xr == 3); // fails, immutable `xr` changed
}

ref can bind to rvalues and escape references to the stack this way

ref int escape() @safe
{
    ref int x = 4; // accepted!
    return x; // accepted!
}

[ntrel]

Perhaps add a -preview switch for this until it's stable enough? Then we can fix bugs separately as they come up.

[WalterBright]

I tried to rebase it, but it got all fouled up by the refactoring done to escape.d. Sigh.

[dkorpel]

I tried to rebase it, but it got all fouled up by the refactoring done to escape.d. Sigh.

You can just remove all changes this PR does to escape.d, it's already accomodating ref variables now.

[WalterBright]

ref can bind to rvalues and escape references to the stack this way

There's already a test for that in diag9679.d. Like I said, the refactor means I have to go back through and refigure out how to implement it.

[dkorpel]

There's already a test for that in diag9679.d. Like I said, the refactor means I have to go back through and refigure out how to implement it.

You just have to check that the initializer is an lvalue in dsymbolsem, escape.d works fine currently no?

[WalterBright]

@JohanEngelen that example isn't part of the dip https://github.com/WalterBright/documents/blob/984374ca885e1cb10c2667cf872aebc13b4c1663/varRef.md

So I'm not sure what you're looking at?

[WalterBright]

Perhaps add a -preview switch for this until it's stable enough?

As this is additive, and doesn't change existing code, it isn't necessary.

[JohanEngelen]

@JohanEngelen that example isn't part of the dip https://github.com/WalterBright/documents/blob/984374ca885e1cb10c2667cf872aebc13b4c1663/varRef.md

So I'm not sure what you're looking at?

My point was to add the examples from the dip and add more. The example I gave is something that seems obvious but prevents bugs in the implementation where auto infers ref (it shouldn't).

[WalterBright]

@JohanEngelen the language never infers storage class, but I put your example in the test suite anyway.

I don't think it needs more, after all, ref is used in several other contexts and has been pretty thoroughly debugged.

[JohanEngelen]

A big reason why there are still so many bugs and regressions in the compiler is because tests are so scarce. The functionality says: "ref can now be applied to local, static, extern, and global variables". Four categories of variables, half of which don't even have a single test case...

[JohanEngelen]

Yep, and as I already predicted: https://issues.dlang.org/show_bug.cgi?id=24729