fix issue 3546 - Support for alias of individual array and aggregate elements

[ghost]

An alias allows to reference a variable. When used this alias to a var is then turned into a VarExp. The idea is why not allowing an extra index, as long as the index is for a variable and that it is a compile-time constant ?

[ntrel]

If you rename the title to 'Fix Issue'... then the bot should automatically link this pull to bugzilla.

[MoonlightSentinel]

The bot doesn't pick up draft PR's IIRC.

[wilzbach]

The bot doesn't pick up draft PR's IIRC.

Yup and it only looks at commit messages, because the changelog will be assembled from commit messages.

[ntrel]

Oh sorry. Seems like the bot linking to bugzilla even for drafts would be useful.

[MoonlightSentinel]

This raises the question why it's limited to index expressions and e.g. disallows function calls.

~~Fun fact: This PR allows aliases to function calls for overloaded opIndex.~~

EDIT: Because it allows overloaded opIndex

[ghost]

Fun fact: This PR allows aliases to function calls for overloaded opIndex.

Yeah, indeed, it's even part of the tests

[MoonlightSentinel]

You're right, overlooked that example.

[MoonlightSentinel]

In a sense you can consider this PR as

alias A = ArrayExpression(ASymbol, Value).

so the alias is allowed because

1. alias Symbol = A.Symbol;
2. enum index = A.Value;

both work. The basic elements of the expression are aliasable so the expression is.

I fully agree with this rule. My concern here is that this PRs adds exceptions, which need to be justified because they break user expectations, e.g.:

int[] a = ....;
int f(int) { ... }
alias b = a[0]; // works
alias c = f(a[0]); // fails?

This does not mean your PR needs to implement everything, it's just necessary to take a look at the bigger picture when adding new features.

[MoonlightSentinel]

Also, this PR should explain why it's better than current workarounds

EDIT: Which propably exists.

[ghost]

current workarounds

There's no. Eventually using functions.

should explain

OK. This saves backend time to inline a function that reads or write from an offset.

[MoonlightSentinel]

There's no. Eventually using functions.

Right, functions / lambdas are kinda akward:

int[2] pipeFd;
pipe(pipeFd);

alias readEnd = pipeFd[0];
alias writeEnd = pipeFd[1];

// <use readEnd, writeEnd>

// Previously
alias readEnd = ref () => pipeFd[0];
alias writeEnd =ref () => pipeFd[1];

// Need parentheses => readEnd(), writeEnd()

OK. This saves backend time to inline a function that reads or write from an offset.

At the expense of more time spent in semantic, so i doubt that there will be much improvement

EDIT: Added ref to those lambdas

[ghost]

you need to test setter and getter overloads coexisting too... ;), i.e read and write using your lambdas and same identifier and without parens ;) like a real access to the thing.

EDIT: rewrite all the "runnable" tests with the lambda alternative.

[MoonlightSentinel]

Added the missing refs

[ghost]

rewrite the tests ;)

[MoonlightSentinel]

(I'm not personally arguing against your PR, just exploring it's rationale and consequences s.t. it can convince others as well)

[ghost]

ok, so

void main(string[] args)
{
    version(none) // current D lang: buzz
    {
///tmp/temp_7F557AF23430.d(11,16): Error: incompatible types for `(__lambda2) == (1337)`: `int delegate() pure nothrow @nogc ref @safe` and `int`
///tmp/temp_7F557AF23430.d(12,9): Error: `rw` is not an lvalue and cannot be modified

        int[1] a = 1337;
        alias rw = ref () => a[0];
        assert(rw == 1337);
        rw = 42;
        assert(a[0] == 42);
    }
    else // with my PR OK
    {
        int[1] a = 1337;
        alias rw = a[0];
        assert(rw == 1337);
        rw = 42;
        assert(a[0] == 42);
    }
}

so test this small program with DMD built with this PR and change version(none) with version(all) to see the difference

[MoonlightSentinel]

As said before, you need to use ~~braces~~ parentheses. See https://run.dlang.io/is/VxqbRN

[ghost]

you need to use braces

Yeah, parens not braces. So this style of alias is not perfect. I tried them before programming what is proposed here. The drawback is that you need to know that the alias is a function, let's say in metaprog code, while with the new shortcut this works transparently as if a variable is read or written (because this is the case).

[MoonlightSentinel]

Good point, easier metaprogramming is always appreciated. (You should probably add a test case for that as well)

[MoonlightSentinel]

FYI: This change will need some form of formal approval

CC @WalterBright

[dlang-bot]

Thanks for your pull request, @NilsLankila!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	3546	enhancement	Aliasing an element of a static array should be legal if the index is a compile time constant
✗	14128	major	AliasDeclaration allows expressions, causing false code for ThisExp

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#11273"

[ghost]

That doesn't work as well as it used to (100% nice Monday, 80% nice today). The problem is that AliasDeclaration.toAlias in some context is not necessarily called to build the INdexExp. When this happens, toAlias has to be refused, so that the offset to apply is not lost. For example in test/fail_compilation/index_alias.d, test7, this is why the second alias is refused.

I'll try to see if a new type of symbol can be added, (e.g a new ASTNode...) so that the original var and the offset are tied more strongly, and also to prevent special cases when processing AliasDeclaration.

[ghost]

I'll try to see if a new type of symbol can be added, (e.g a new ASTNode...) so that the original var and the offset are tied more strongly

This has worked. No more special case and alias of an alias of "offset symbol" works now.

[pbackus]

Allowing aliases to refer to expressions like this is equivalent to adding AST macros.

Consider the following program, which compiles, runs, and prints the indicated output with the changes proposed in this PR:

template counter()
{
    struct Counter
    {
        static size_t count;
        size_t next() { return count++; }
    }

    alias counter = Counter.init.next;
}

void main()
{
    import std.stdio: writeln;

    writeln(counter!()); // 0
    writeln(counter!()); // 1
    writeln(counter!()); // 2
}

This should not be merged without a DIP.

[ghost]

awesome discovery @pbackus, I really like this example. But yes you're right it's a sort of macro system / expression templates, as in plenty of contexts the source expression is syntax-copied.

[tgehr]

Allowing aliases to refer to expressions like this is equivalent to adding AST macros.

Consider the following program, which compiles, runs, and prints the indicated output with the changes proposed in this PR:

template counter()
{
    struct Counter
    {
        static size_t count;
        size_t next() { return count++; }
    }

    alias counter = Counter.init.next;
}

void main()
{
    import std.stdio: writeln;

    writeln(counter!()); // 0
    writeln(counter!()); // 1
    writeln(counter!()); // 2
}

This should not be merged without a DIP.

Well, this works today:

template counter()
{
    struct Counter
    {
        static size_t count;
	static size_t next() { return count++; }
    }

    alias counter = Counter.next;
}

void main()
{
    import std.stdio: writeln;

    writeln(counter!()); // 0
    writeln(counter!()); // 1
    writeln(counter!()); // 2
}

I don't think the example you show demonstrates anything like AST macros.