Issue 11169 - __traits(isAbstractClass) prematurely sets a class to be abstract #5695
Conversation
|
| @@ -273,7 +273,7 @@ class ClassDeclaration : public AggregateDeclaration | |||
| bool com; // true if this is a COM class (meaning it derives from IUnknown) | |||
| bool cpp; // true if this is a C++ interface | |||
| bool isscope; // true if this is a scope class | |||
| bool isabstract; // true if abstract class | |||
| int isabstract; // 0: fwdref, 1: is abstract class, 2: not abstract | |||
There was a problem hiding this comment.
Whenever a value has more than two states, I'd really prefer it to be an enum.
There was a problem hiding this comment.
Also, isabstract becomes a bad name when it is tri-state, because if (isabstract) will be true even if isabstract is 2 !
There was a problem hiding this comment.
Changed to use enum.
9rnsr
force-pushed
the
fix11169
branch
2 times, most recently
from
54392cc
to
860c01e
Compare
| @@ -8715,6 +8715,25 @@ public: | |||
| return 0; | |||
| } | |||
|
|
|||
| override int apply2(Dsymbol_apply_ft_t fp, void* param) | |||
There was a problem hiding this comment.
My problem with this PR is the addition of apply2 which is the same only different from apply. I don't know why one should work on TemplateMixins and the other not. The fact that there's a difference needs to be explained, it looks like a bug.
There was a problem hiding this comment.
OK I checked whole dmd code. Now Dsymbol.apply is used only in:
AggregateDeclaration.determineFields
AggregateDeclaration.searchCtor()
And they are usually invoked after the scope members' semantic finished, so today they would already see resolved template mixin members.
Therefore, resolving TemplateMixin in its apply would have low risk to introduce new forward reference issues.
Let's wait auto-tester result.
There was a problem hiding this comment.
Thank you. This is more like it!
…to be abstract __traits(isAbstractClass) should resolve forward references of all class member functions, in order to return stable result.
|
Auto-merge toggled on |
|
Digger says this introduced a regression: https://issues.dlang.org/show_bug.cgi?id=16273 |
| return 0; | ||
|
|
||
| if (fd._scope) | ||
| fd.semantic(null); |
There was a problem hiding this comment.
This causes a forward reference bug, https://issues.dlang.org/show_bug.cgi?id=16273
__traits(isAbstractClass)should resolve forward references of all class member functions, in order to return stable result.