dmd.aggregate: Define importAll override for AggregateDeclaration

[ibuclaw]

The laziness of adding aggregate members to the symbol table is a source
of the dozens of forward reference bugs in D. This change makes it so
that known members are added in before running semantic to give them the
best possible chance of being resolved without needing to trigger a
cascading effect of `semantic > resolve > semantic > resolve > ...`
which can wrongly trip up the "forward reference to ..." detection if
declarations appear in the source file in the wrong order.

I would have preferred to add importAll to a lot more AST nodes, but
there is a lot of unpicking to be done in various semantic visitor
passes in order to properly support early symbol resolving, and maybe
places use `!symtab` to determine whether semantic should be ran, to
which modifying has the potential for breaking downstream code if not
handled right, despite fixing half a dozen or more. In some cases there
are simply too many knots to untangle, but this seems like a good start
and is relatively small.

[dlang-bot]

Thanks for your pull request, @ibuclaw!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	10616	normal	forward reference error with `class C: C.D{static class D{}}`
✓	20913	critical	Array "forward reference" error
✓	22981	normal	Another forward reference bug involving a string mixin
✓	23595	blocker	Error: undefined identifier with static if and module/import order
✓	23598	blocker	Another nasty forward reference bug

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

• Regression or critical bug fixes should always target the stable branch. Learn more about rebasing to stable or the D release process.

To target stable perform these two steps:

1. Rebase your branch to upstream/stable:

git rebase --onto upstream/stable upstream/master

2. Change the base branch of your PR to stable

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

[ghost]

A regression:

mixin template ComponentImpl()
{
    alias DispatchTable = int;
}

struct DispatchTableInfo
{
    alias foo = Top.DispatchTable;
}

struct Top
{
    mixin ComponentImpl;
}

Error: no property DispatchTable for type main.Top

The test compiles if DispatchTableInfo is placed after Top.

[ibuclaw]

A regression:

Yeah, I fear the only way forward is a more invasive architectural change. I was just looking at this apparent regression too:

static if (true)
    version = FOO;  // Error: defined after use

struct Bar
{
    version (FOO)
        int a;
    int b;
}

Having a version = ... inside a static if should really just be a compile-time error, same as static foreach().

[ghost]

Even if you special-case 'version', there is still this:

struct Bar
{
    static if (is(typeof(FOO)))
        int a;
    int b;
}

static if (true)
    enum FOO = 1;

void main()
{
    Bar b; b.a = 1; // error
}

[ibuclaw]

Even if you special-case 'version', there is still this:

struct Bar
{
    static if (is(typeof(FOO)))
        int a;
    int b;
}

static if (true)
    enum FOO = 1;

void main()
{
    Bar b; b.a = 1; // error
}

Right, I am in no way trying to address the tangle mess that static ifs add, only aid in untangling the resolving of what is unconditionally known.

Dealing with static ifs unless there is a clear path to organising out of order resolving will require something of a complete and total rewrite of everything. At the moment there's a lot of components that don't communicate what has changed through the process.

• code expansion: debug/versions are early; static if/foreach, templates and mixins are late.
• symbol resolving: picks most publicly visible symbol
• alias resolving: discarded early, loosing any attribute information attached to them (such as public alias to private template)
• property resolving: adjusts symbol/expressions post-alias resolving
• function call matching: adjusts currently resolved symbol to the correct overload one that matches call args (ignores visibility, too late to do a post-check here)

[ghost]

Ok, thanks for the explanation.

[RazvanN7]

@ibuclaw is there any way we can move this forward?