Thread overview
[Issue 5110] New: Excess attribute propagation of structs and classes
Oct 24, 2010
Shin Fujishiro
Oct 24, 2010
Shin Fujishiro
Oct 24, 2010
Shin Fujishiro
Oct 24, 2010
Shin Fujishiro
Dec 05, 2010
Walter Bright
Dec 05, 2010
Walter Bright
Dec 05, 2010
Walter Bright
October 24, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=5110

           Summary: Excess attribute propagation of structs and classes
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Keywords: spec
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: nobody@puremagic.com
        ReportedBy: rsinfu@gmail.com


--- Comment #0 from Shin Fujishiro <rsinfu@gmail.com> 2010-10-24 02:02:08 PDT ---
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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
October 24, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=5110



--- Comment #1 from Shin Fujishiro <rsinfu@gmail.com> 2010-10-24 02:05:22 PDT ---
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).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
October 24, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=5110



--- Comment #2 from Shin Fujishiro <rsinfu@gmail.com> 2010-10-24 02:09:16 PDT ---
Created an attachment (id=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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
October 24, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=5110



--- Comment #3 from Shin Fujishiro <rsinfu@gmail.com> 2010-10-24 02:18:33 PDT ---
Created an attachment (id=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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
December 05, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=5110


Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |bugzilla@digitalmars.com
         Resolution|                            |FIXED


--- Comment #4 from Walter Bright <bugzilla@digitalmars.com> 2010-12-05 12:56:06 PST ---
http://www.dsource.org/projects/dmd/changeset/781

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
December 05, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=5110


Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tomeksowi@gmail.com


--- Comment #5 from Walter Bright <bugzilla@digitalmars.com> 2010-12-05 13:00:05 PST ---
*** Issue 3598 has been marked as a duplicate of this issue. ***

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
December 05, 2010
http://d.puremagic.com/issues/show_bug.cgi?id=5110


Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |zan77137@nifty.com


--- Comment #6 from Walter Bright <bugzilla@digitalmars.com> 2010-12-05 13:03:18 PST ---
*** Issue 4211 has been marked as a duplicate of this issue. ***

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------