[REG 2.066] Issue 14838 - Wrong attribute inference for auto-generated class destructor with static array of non-POD type #4845
Conversation
| "void _ArrayPostblit(T)(T[] a) { foreach (ref T e; a) e.__xpostblit(); }\n"; | ||
|
|
||
| static const utf8_t code_ArrayDtor[] = | ||
| "void _ArrayDtor(T)(T[] a) { foreach_reverse (ref T e; a) e.__xdtor(); }\n"; |
There was a problem hiding this comment.
We already have those in druntime, called _recursiveDestroy and _recursivePostblit, but it's indeed an interesting idea to move them into the compiler.
There was a problem hiding this comment.
Do you mean _destructRecurse and _postblitRecurse? I also planned them, but i decided not to use them.
Because they're specialized to T[n]. It will instantiate different templates for T[1], T[2], ..., so they would cause template bloat.
There was a problem hiding this comment.
I agree, your solution is much nicer. Also we no longer need those functions in druntime, better if everything stays on one side.
dlang/druntime#1335
|
Mmh, seems like I shouldn't have spent my time on this (MartinNowak@3a149d2), maybe you find something interesting. |
| p.nextToken(); | ||
| members->append(p.parseDeclDefs(0)); | ||
| } | ||
| } |
There was a problem hiding this comment.
Why does it have to be module object? How about module __internals and then parsing a single piece of code.
That way you wouldn't need to deal with conflicting symbols.
There was a problem hiding this comment.
It would be better, but this is the most easy way that I can implement quickly. This change is for the regression issue, so I should not use risky way.
There was a problem hiding this comment.
A separate module might require linkage, but appending to object might also cause issues and sure is confusing. But you're right, let's do that as follow-up.
|
Mostly looks good, thanks a lot. Will continue the review this evening. |
By this, we can avoid the mutual dependency between compiler and druntime.
…destructor with static array of non-POD type
[REG 2.066] Issue 14838 - Wrong attribute inference for auto-generated class destructor with static array of non-POD type
| * Need to add things like "const ~this()" and "immutable ~this()" to | ||
| * fix properly. | ||
| */ | ||
| e->type = e->type->mutableOf(); |
There was a problem hiding this comment.
Here you are overwriting e->type. Why is that different from the case above?
There was a problem hiding this comment.
Because VarExp is one of the leaf AST node. Its semantic will return itself. So the type setting is low risk.
|
@MartinNowak Thanks! |
|
I've alreay seen a lot of error message à la |
|
Ping |
|
Internal module is not bad idea, but I'm not sure which name is the best for that module ( Better approach for the issue is to think about the ideal diagnostic message what you expects. Please give me some idea If you have. |
|
This introduced a regression https://issues.dlang.org/show_bug.cgi?id=15304 |
https://issues.dlang.org/show_bug.cgi?id=14838
Add
_ArrayPostblitand_ArrayDtor, then use them for static array copying and destruction.