Patch against dmd r727, passed dmd/druntime/phobos tests
text/plain
3532
Comments
Comment #0 by rsinfu — 2010-10-24T02:02:08Z
The override attribute is unnecessarily propagated to a nested class declaration and causes errors:
--------------------
class C
{
override:
string toString() { return ""; }
class Nested
{ // (7)
void gun() {} // (8)
}
}
--------------------
test.d(8): Error: function test.C.Nested.gun does not override any function
test.d(7): Error: variable test.C.Nested.this override cannot be applied to variable
--------------------
Another case. The const attribute is propagated to a static member:
--------------------
const struct S
{
static int value;
}
static assert(is(typeof(S.value) == int)); // (5)
--------------------
test.d(5): Error: static assert (is(const(int) == int)) is false
--------------------
Though the spec allows this behavior, I think it's more natural if the static member variable S.value is typed as mutable int.
Comment #1 by rsinfu — 2010-10-24T02:05:22Z
The current ClassDeclaration uses the following black list for masking attributes to propagate over its members:
--------------------
sc->stc &= ~(STCfinal | STCauto | STCscope | STCstatic |
STCabstract | STCdeprecated | STC_TYPECTOR | STCtls | STCgshared);
sc->stc |= storage_class & STC_TYPECTOR;
--------------------
The following STCs pass the black list:
STCextern, STCparameter, STCfield, STCoverride, STCsynchronized,
STCin, STCout, STClazy, STCforeach, STCcomdat, STCvariadic,
STCctorinit, STCtemplateparameter, STCref, STCinit, STCmanifest,
STCnodtor, STCnothrow, STCpure, STCalias, STCproperty, STCsafe,
STCtrusted, STCsystem, STCctfe, STCdisable, STCresult.
Currently, the STCs above are propagated inside a class. Some STCs such as STCparameter and STCout cannot be applied to class declarations; removing such insignificant STCs, I got the following list:
STCextern, STCoverride, STCsynchronized, STCref, STCnothrow,
STCpure, STCproperty, STCsafe, STCtrusted, STCsystem, STCdisable.
Among them, the only STCs with which propagation makes sense are:
STCsynchronized, STCnothrow, STCpure, STCsafe, STCtrusted,
STCsystem, STCdisable.
Other STCs such as STCoverride should not be propagated (the reported problem).
Comment #2 by rsinfu — 2010-10-24T02:09:16Z
Created attachment 792
Testcases
Since nothrow, pure and @disable are one-way, non-revertible attributes, the language should not force them to be propagated IMO. So, structs and classes should really propagate only the following STCs:
STCimmutable, STCconst, STCshared, STCsynchronized,
STCsafe, STCtrusted, STCsystem.
Note that the first four STCs must not be applied to static members, including nested types:
--------------------
const synchronized class C
{
static int value; // NO const
enum E { a, b, c } // NO const
class N {} // NO const synchronized
}
--------------------
The nested class N itself isn't necessarily const nor synchronized, since its declaration is effectively the same as the following one. Whether N should be const/synchronized or not is independent of C.
--------------------
class N
{
C outer; // C is already const synchronized
}
--------------------
To sum up, the rule allows these attribute propagations:
- const, shared, immutable and synchronized over non-static members
- @safe, @trusted and @system over all members
Attached test cases.
Comment #3 by rsinfu — 2010-10-24T02:18:33Z
Created attachment 793
Patch against dmd r727, passed dmd/druntime/phobos tests
The proposed patch implements the said rule.
The patch limits STC propagation via Scope's storage class (sc->stc) only to @safe/@trusted/@system. Other STCs such as const and synchronized are 'pulled' by need of each member out of parent AggregateDeclaration.
This patch also fixes bug 3598 and bug 4211.