fix Issue 9244 - union containing pointers not allowed

[WalterBright]

fix http://d.puremagic.com/issues/show_bug.cgi?id=9244

[9rnsr]

Your intent is: "union declarations cannot become nested, even if it is declared inside function". Right?

[9rnsr]

I think that itself is a right direction, but the fact that such a union member functions cannot access to enclosing context is inconsistent with normal nested structs. At least we need to update documentation about that.

[WalterBright]

Unions cannot have a context pointer - there's no place for them.

[yebblies]

It should produce an error like "union declarations can not be nested", not the generic "cannot access frame". Of course in this case the error will be bogus, because:
It shouldn't be trying to make the union nested in the first place, as it doesn't rely on any of the enclosing frame's data.

If this is just a quick hack to get it off the regression list then there should at least be a comment in makeNested to that effect.

[WalterBright]

The code example compiles fine. It doesn't produce an error message that needs improving.

[yebblies]

The generic error appears with code like this:

void main()
{
    int i;
    union U { int x() { return i; } }
}

testx.d(5): Error: function testx.main.U.x cannot access frame of function D main

[WalterBright]

Ok, and I don't think that's a broken error message.

[9rnsr]

Unions cannot have a context pointer - there's no place for them.

@WalterBright Yes, it is one of the consistent concepts. But in this case, we can take one another concept: if we need to make a union nested, compiler adds hidden context pointer without any overlapping with other fields.

The real issue which I think is: which concept should be respected:

1. union declaration cannot have non-overlapped field, even if it is the hidden context pointer.
2. an aggregate type which declared inside function should make a nested type.

This is an important language design decision. You decided taking #1 from the standpoint of a compiler implementer. Of course there is no problem, but on the other hand, #2 is still a worthy choice from the standpoint of a language designer.

To @andralex : I'd like to hear your opinion.

[9rnsr]

Don't forget that compiler can control the offset of a hidden context pointer specially.

[yebblies]

@WalterBright It's not incorrect, just incredibly unhelpful.

[9rnsr]

I think it is better to enforce writing "static", rather than implicitly changing function-local unions to un-nested.

void foo() {
    union U1 { int x; void x(){} }  // Error: union declaration cannot be nested, please add "static"
    static union U2 { int x; void x(){} }  // OK
}

[WalterBright]

Unions conceptually cannot have a context pointer. It doesn't even make any sense. Even having any member functions in a union is of dubious usefulness. 9rnsr, you say I am saying this as a compiler guy. But I have yet to see any legitimate use of unions with a context pointer, and if one needs one, it is pretty trivial to put the union inside a struct.

The only real purpose of a union is to provide a means of overlapping fields when laying out a struct.

yebblies, I disagree that the error message is "incredibly unhelpful". If people are going to use nested references, they need to understand what a frame is, otherwise they are just going to be lost anyway. I prefer error messages to accurately describe the situation, rather than dance around it and be an impediment to understanding.

It would be like trying to use pointers with no understanding of what an address is, and I've run into people with that problem. No rejiggering of error messages can help them.

As for this being a regression, there never was a context pointer put in unions, so such code never would have worked anyway, even if it compiled. A redesign of unions to have a context pointer would be an enhancement request, and would need some fairly compelling use cases to justify it.

[WalterBright]

9rnsr, in reply to your suggestion that 'static' must be applied, I think that is based on the notion that unions are simply a special case of structs. While they are like that in the compiler, I see a struct and a union as being fairly different, like structs and classes are. Since I do not see a case for a nested union, requiring 'static' in front of the union is redundant.

[WalterBright]

In fact, it's debatable whether unions should even exist as a type, rather than just as something like an alignment specification within a struct.

[alexrp]

yebblies, I disagree that the error message is "incredibly unhelpful". If people are going to use nested references, they need to understand what a frame is, otherwise they are just going to be lost anyway.

Yes, most people know what a stack frame is. But that does not matter at all.

The problem here is that the way lexical closures are implemented is completely implementation-specific (and even then not very obvious). You'd need to know how DMD implements them to know why "frame" is even relevant to the error. Most people don't.

The error should not assume that someone is so familiar with compiler internals.

[9rnsr]

Since I do not see a case for a nested union

OK. I understand. In this regard, I agree that you have more experience than I.

requiring 'static' in front of the union is redundant.

If we agree that a union conceptually has no context pointer, omitting redundant "static" is reasonable. But if someone want my enhancement (yes, I finally agree that it is a kind of enhancement), the omission of "static" will forbid to extend union semantics in the future. It is difficult...

As a side note, I noticed that an interface declaration has no context pointer, even if it is declared inside function.

[9rnsr]

Hmm, enforcing "static union" will avoid some confusion/misunderstanding such as @alexrp described. I'd like to vote to it.

[yebblies]

@WalterBright
I think I see where you're coming from.

There are two things the use might want to know when they encounter this error:

1. What exactly is the code doing wrong?
   'cannot access frame ...' answers that.
2. Why can't it access the frame?
   'unions cannot be nested' answers this one.

This first one is useful when the union was accidentally made nested, and the second when the user didn't realize nested unions didn't work.

Do you agree that the second message is useful?

[donc]

I like Kenji's idea of requiring 'static'. The error message "nested unions must be static" would be completely clear. I agree that a non-static union doesn't make sense and should never be implemented, but for clarity and nice error messages, it's worth requiring this extra bit of verbosity for this really obscure corner case.

[9rnsr]

Just a memo: I wrote a small patch to allow "nested union".
https://gist.github.com/4690822

[dnadlinger]

I started a discussion about this on dmd-internals some months ago (please ignore the hilarious amount of typos in my last post there): [dmd-internals] Nested Unions?

In the meantime, LDC has been shipping with the following sledgehammer-type patch, which just implicitly marks unions as static: ldc-developers/ldc@a9e151e.

Even though we generally try to avoid LDC-specific frontend changes, I had to do something about the issue, because with LLVM IR being typed, nested unions with member methods would lead to an assertion during codegen for the same reason they fail at runtime with DMD (and there is such a case in the Phobos test suite, an union declared inside a format unittest block). Just implicitly making them static seemed like the best way to go, and there hasn't been a single complaint about the change yet.

Diagnostics on the issue can certainly be improved, but note that about 99.99% of all D users will never ever attempt to do something like that.

[WalterBright]

alexrp, it is not about compiler internals, any more than "address" is about compiler internals.

9rnsr, in the future, a nested union could be enhanced to contain a context pointer. I don't think the current design would close the door on that.

[ghost]

Even having any member functions in a union is of dubious usefulness.

It's useful as documentation when you're forced to use property functions when interfacing with C++.

E.g. pseudo-C++ example:

class C  // e.g. some polymorphic C++ class
{
    union
    {
        int intval;
        int_8 shortval;
    };
};

D side:

class C
{
    union
    {
        @property void intval(int);
        @property int intval(int);

        @property void shortval(short);
        @property int shortval(short);
    }
}

[WalterBright]

I don't see a compelling reason to put those member functions inside the union rather than the class.