www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - dup field in sub-class should be reported by the compiler

reply name <someanon yahoo.com> writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1419

Description:  [reply]  	 Opened: 2007-08-13 13:24

dupdefbug.d:
================================
class A {
  int attr;
}

class B : A {
  int attr;
}
================================

$ dmd.exe -c dupdefbug.d

no error reported.

It should report B.attr is dup of A.attr.




Why should it be an error? Fields are not accessed virtually, so there is no
hijacking problem. The current behavior is as designed.

If you feel it should be changed, please start a thread in digitalmars.D and
present the case.


==================================================================
I think 90% of the time this happens it is a bug, not by the programmer's
intention.

If it's designed to be so in D, I propose to change it; or at least reported
by the compiler.
Aug 13 2007
next sibling parent "Vladimir Panteleev" <thecybershadow gmail.com> writes:
On Mon, 13 Aug 2007 21:45:28 +0300, name <someanon yahoo.com> wrote:


 I think 90% of the time this happens it is a bug, not by the programmer's
 intention.

 If it's designed to be so in D, I propose to change it; or at least reported
 by the compiler.


Agreed - with non-private fields only, of course.

-- 
Best regards,
  Vladimir                          mailto:thecybershadow gmail.com
Aug 13 2007
prev sibling parent reply sa <someanon yahoo.com> writes:
== Quote from name (someanon yahoo.com)'s article

 http://d.puremagic.com/issues/show_bug.cgi?id=1419
 Description:  [reply]  	 Opened: 2007-08-13 13:24
 dupdefbug.d:
 ================================
 class A {
   int attr;
 }
 class B : A {
   int attr;
 }
 ================================
 $ dmd.exe -c dupdefbug.d
 no error reported.
 It should report B.attr is dup of A.attr.

 Why should it be an error? Fields are not accessed virtually, so there is no
 hijacking problem. The current behavior is as designed.
 If you feel it should be changed, please start a thread in digitalmars.D and
 present the case.
 ==================================================================
 I think 90% of the time this happens it is a bug, not by the programmer's
 intention.
 If it's designed to be so in D, I propose to change it; or at least reported
 by the compiler.


This is particularly true in a large system where multiple people working on it,
the programmer may simply forget what's defined in the super class; and when
this
 cause a program error (most of the time to the programmer's surpise), it may
took
several hours to find.

Since Walter want to discuss this issue on this NG, audience pleaes air your
voice.
Aug 13 2007
next sibling parent Extrawurst <spam extrawurst.org> writes:
i agree, it should be at least a warning for shadowing non-private 
fields of the super class.


sa schrieb:

 == Quote from name (someanon yahoo.com)'s article
   

 http://d.puremagic.com/issues/show_bug.cgi?id=1419
 Description:  [reply]  	 Opened: 2007-08-13 13:24
 dupdefbug.d:
 ================================
 class A {
   int attr;
 }
 class B : A {
   int attr;
 }
 ================================
 $ dmd.exe -c dupdefbug.d
 no error reported.
 It should report B.attr is dup of A.attr.

 Why should it be an error? Fields are not accessed virtually, so there is no
 hijacking problem. The current behavior is as designed.
 If you feel it should be changed, please start a thread in digitalmars.D and
 present the case.
 ==================================================================
 I think 90% of the time this happens it is a bug, not by the programmer's
 intention.
 If it's designed to be so in D, I propose to change it; or at least reported
 by the compiler.
     


 This is particularly true in a large system where multiple people working on
it,
 the programmer may simply forget what's defined in the super class; and when
this
  cause a program error (most of the time to the programmer's surpise), it may
took
 several hours to find.

 Since Walter want to discuss this issue on this NG, audience pleaes air your
voice.
Aug 13 2007
prev sibling parent reply Chris Nicholson-Sauls <ibisbasenji gmail.com> writes:
sa wrote:

 == Quote from name (someanon yahoo.com)'s article

 http://d.puremagic.com/issues/show_bug.cgi?id=1419
 Description:  [reply]  	 Opened: 2007-08-13 13:24
 dupdefbug.d:
 ================================
 class A {
   int attr;
 }
 class B : A {
   int attr;
 }
 ================================
 $ dmd.exe -c dupdefbug.d
 no error reported.
 It should report B.attr is dup of A.attr.

 Why should it be an error? Fields are not accessed virtually, so there is no
 hijacking problem. The current behavior is as designed.
 If you feel it should be changed, please start a thread in digitalmars.D and
 present the case.
 ==================================================================
 I think 90% of the time this happens it is a bug, not by the programmer's
 intention.
 If it's designed to be so in D, I propose to change it; or at least reported
 by the compiler.


 
 This is particularly true in a large system where multiple people working on
it,
 the programmer may simply forget what's defined in the super class; and when
this
  cause a program error (most of the time to the programmer's surpise), it may
took
 several hours to find.
 
 Since Walter want to discuss this issue on this NG, audience pleaes air your
voice.


Or, as has happened to me a time or two, the programmer may not even /know/
what's in the 
super class, if deriving from a class provided by a library for example.  (This
is 
particularly true with private fields, of course.)

I'd favor making it a warning in the case of public/protected fields, and an
error in the 
case of package/private fields.

-- Chris Nicholson-Sauls
Aug 13 2007
next sibling parent BCS <ao pathlink.com> writes:
Reply to Chris Nicholson-Sauls,


 
 I'd favor making it a warning in the case of public/protected fields,
 and an error in the case of package/private fields.
 


why that way? I'd expect the other way around.
Aug 13 2007
prev sibling parent reply Walter Bright <newshound1 digitalmars.com> writes:
Chris Nicholson-Sauls wrote:

 Or, as has happened to me a time or two, the programmer may not even 
 /know/ what's in the super class, if deriving from a class provided by a 
 library for example.  (This is particularly true with private fields, of 
 course.)


Exactly what problem does this solve? If you're writing a derived class, 
and you create another field of the same name as a field in the base 
class, what bug has been created? The base class functions will still 
use the base class field, the derived class functions will still use the 
derived class field. The fields aren't polymorphic, so no hijacking can 
happen.

I just don't see the issue.
Aug 13 2007
next sibling parent reply sa <someanon yahoo.com> writes:
 Exactly what problem does this solve? If you're writing a derived class,
 and you create another field of the same name as a field in the base
 class, what bug has been created? The base class functions will still
 use the base class field, the derived class functions will still use the
 derived class field. The fields aren't polymorphic, so no hijacking can
 happen.
 I just don't see the issue.


It's less a problem if the programmer is *aware* of the re-definition; but in
practice, most the time, the programmer just forget, and will *assume* there's
only one definition, then it will cause strange error in his eye.

Now, even from an OO modelling point of view:

Dog {
  Tail tail;
}

MyDog {
  Tail tail;
}

Why should MyDog have a tail which is already defined in Dog?

It burden the programmer's thoughts, instead of simplify it.
Aug 13 2007
parent sa <someanon yahoo.com> writes:
== Quote from sa (someanon yahoo.com)'s article

 Exactly what problem does this solve? If you're writing a derived class,
 and you create another field of the same name as a field in the base
 class, what bug has been created? The base class functions will still
 use the base class field, the derived class functions will still use the
 derived class field. The fields aren't polymorphic, so no hijacking can
 happen.
 I just don't see the issue.


 It's less a problem if the programmer is *aware* of the re-definition; but in
 practice, most the time, the programmer just forget, and will *assume* there's
 only one definition, then it will cause strange error in his eye.
 Now, even from an OO modelling point of view:
 Dog {
   Tail tail;
 }
 MyDog {
   Tail tail;
 }
 Why should MyDog have a tail which is already defined in Dog?
 It burden the programmer's thoughts, instead of simplify it.


Or during code refactoring, the programmer moved the field to the parent class,
but forget to delete it in the sub class.

The language should be defined to make it harder for the programmer to run into
such error.  So at least it should warn the user.
Aug 13 2007
prev sibling next sibling parent reply Robert Fraser <fraserofthenight gmail.com> writes:
Walter Bright Wrote:


 Chris Nicholson-Sauls wrote:

 Or, as has happened to me a time or two, the programmer may not even 
 /know/ what's in the super class, if deriving from a class provided by a 
 library for example.  (This is particularly true with private fields, of 
 course.)


 
 Exactly what problem does this solve? If you're writing a derived class, 
 and you create another field of the same name as a field in the base 
 class, what bug has been created? The base class functions will still 
 use the base class field, the derived class functions will still use the 
 derived class field. The fields aren't polymorphic, so no hijacking can 
 happen.
 
 I just don't see the issue.
 


I think the problem is with things like this:

class A
{
   string x;
   string y;

   this()
   {
       x = "A.x";
       y = "A.y";
   }
}

class B : A
{
   string x;

  this()
   {
       x = "B.x";
   }
}

public void main(string[] args)
{
   B b = new B();
   A a = b; // a and b refer to the same object
   writefln("A.x = %s", a.x);
   writefln("B.x = %s", b.x);
   writefln("A.y = %s", a.y);
   writefln("B.y = %s", b.y);
}

If A and B are maintained by different people, and the maintainer of B adds a
new field "y", the main code can fail silently, which is, as you call it,
"hijacking".
Aug 13 2007
next sibling parent Sean Kelly <sean f4.ca> writes:
Robert Fraser wrote:

 Walter Bright Wrote:
 

 Chris Nicholson-Sauls wrote:

 Or, as has happened to me a time or two, the programmer may not even 
 /know/ what's in the super class, if deriving from a class provided by a 
 library for example.  (This is particularly true with private fields, of 
 course.)


 Exactly what problem does this solve? If you're writing a derived class, 
 and you create another field of the same name as a field in the base 
 class, what bug has been created? The base class functions will still 
 use the base class field, the derived class functions will still use the 
 derived class field. The fields aren't polymorphic, so no hijacking can 
 happen.

 I just don't see the issue.


 
 I think the problem is with things like this:
 
 class A
 {
    string x;
    string y;
 
    this()
    {
        x = "A.x";
        y = "A.y";
    }
 }
 
 class B : A
 {
    string x;
 
   this()
    {
        x = "B.x";
    }
 }
 
 public void main(string[] args)
 {
    B b = new B();
    A a = b; // a and b refer to the same object
    writefln("A.x = %s", a.x);
    writefln("B.x = %s", b.x);
    writefln("A.y = %s", a.y);
    writefln("B.y = %s", b.y);
 }
 
 If A and B are maintained by different people, and the maintainer of B adds a
new field "y", the main code can fail silently, which is, as you call it,
"hijacking".


If methods were non-virtual by default I don't think there would be a 
case for this suggestion, but as it is, the idea kind of makes sense. 
Private methods are non-virtual by default (and cannot be made virtual), 
so it is even reasonable that this restriction should be enforced only 
for non-private member data.  This is really an extension of the 
"shadowing declarations are illegal" idea, which attempts to prevent 
maintenance-related mistakes from occurring.


Sean
Aug 13 2007
prev sibling parent reply Walter Bright <newshound1 digitalmars.com> writes:
Robert Fraser wrote:

 I think the problem is with things like this:
 
 class A
 {
    string x;
    string y;
 
    this()
    {
        x = "A.x";
        y = "A.y";
    }
 }
 
 class B : A
 {
    string x;
 
   this()
    {
        x = "B.x";
    }
 }
 
 public void main(string[] args)
 {
    B b = new B();
    A a = b; // a and b refer to the same object
    writefln("A.x = %s", a.x);
    writefln("B.x = %s", b.x);
    writefln("A.y = %s", a.y);
    writefln("B.y = %s", b.y);
 }
 
 If A and B are maintained by different people, and the maintainer of B
 adds a new field "y", the main code can fail silently, which is, as you
 call it, "hijacking".



It's not hijacking at all, it's overriding. a.y does not change, i.e. it 
is not hijacked by adding a definition to B. Only b.y changes, and 
that's unsurprising.
Aug 13 2007
next sibling parent James Dennett <jdennett acm.org> writes:
Walter Bright wrote:

 Robert Fraser wrote:

 I think the problem is with things like this:

 class A
 {
    string x;
    string y;

    this()
    {
        x = "A.x";
        y = "A.y";
    }
 }

 class B : A
 {
    string x;

   this()
    {
        x = "B.x";
    }
 }

 public void main(string[] args)
 {
    B b = new B();
    A a = b; // a and b refer to the same object
    writefln("A.x = %s", a.x);
    writefln("B.x = %s", b.x);
    writefln("A.y = %s", a.y);
    writefln("B.y = %s", b.y);
 }

 If A and B are maintained by different people, and the maintainer of B
 adds a new field "y", the main code can fail silently, which is, as you
 call it, "hijacking".


 
 
 It's not hijacking at all, it's overriding. a.y does not change, i.e. it
 is not hijacked by adding a definition to B. Only b.y changes, and
 that's unsurprising.


Not to one surprised by code in B that used to refer to A.x
and now doesn't.  It's a well-known problem, and C++ compilers
mostly include at least an option to warn about it, as users
find that helpful.

Certainly, sufficient care when making changes can avoid this
problem, but then "sufficient care" can avoid all bugs; tools
should help.

-- James
Aug 13 2007
prev sibling parent Robert Fraser <fraserofthenight gmail.com> writes:
Walter Bright Wrote:


 It's not hijacking at all, it's overriding. a.y does not change, i.e. it 
 is not hijacked by adding a definition to B. Only b.y changes, and 
 that's unsurprising.
 


Ah, I see, I misunderstood whatb you meant by hijacking. That said, if it's not
too hard, it seems to be an important issue for people... so could we at least
have a warning?
Aug 14 2007
prev sibling next sibling parent reply Brad Roberts <braddr puremagic.com> writes:
On Mon, 13 Aug 2007, Walter Bright wrote:


 Chris Nicholson-Sauls wrote:

 Or, as has happened to me a time or two, the programmer may not even /know/
 what's in the super class, if deriving from a class provided by a library
 for example.  (This is particularly true with private fields, of course.)


 
 Exactly what problem does this solve? If you're writing a derived class, and
 you create another field of the same name as a field in the base class, what
 bug has been created? The base class functions will still use the base class
 field, the derived class functions will still use the derived class field. The
 fields aren't polymorphic, so no hijacking can happen.
 
 I just don't see the issue.


Apply this same logic to variable shadowing.  Shadowing has been declared 
illegal.
Aug 13 2007
parent reply Walter Bright <newshound1 digitalmars.com> writes:
Brad Roberts wrote:

 On Mon, 13 Aug 2007, Walter Bright wrote:

 I just don't see the issue.


 
 Apply this same logic to variable shadowing.  Shadowing has been declared 
 illegal.


While pedantically it is the same logic, but practically I think it's 
very different. I don't see writing derived classes like one writes a 
nested scope.
Aug 13 2007
parent reply Walter Bright <newshound1 digitalmars.com> writes:
Walter Bright wrote:

 Brad Roberts wrote:

 On Mon, 13 Aug 2007, Walter Bright wrote:

 I just don't see the issue.


 Apply this same logic to variable shadowing.  Shadowing has been 
 declared illegal.


 
 While pedantically it is the same logic, but practically I think it's 
 very different. I don't see writing derived classes like one writes a 
 nested scope.


I want to add to that. For a nested scope, there really isn't any 
barrier to renaming something. For a derived class, however, you have 
users of it, and you may need to specifically hide a base member.
Aug 13 2007
next sibling parent sa <someanon yahoo.com> writes:
 I want to add to that. For a nested scope, there really isn't any
 barrier to renaming something. For a derived class, however, you have
 users of it, and you may need to specifically hide a base member.


How often does that happen?

And how often people run into subtle bugs that cause by such re-definition?
(count
the replies in the this thread)
Aug 13 2007
prev sibling next sibling parent Manfred Nowak <svv1999 hotmail.com> writes:
Walter Bright wrote


 you may need to specifically hide a base member.


If one needs to hide a base member, why not use composition?
For what is composition to be used in the philosophy of D, if it can be 
emulated by overriding?

-manfred
Aug 14 2007
prev sibling parent Regan Heath <regan netmail.co.nz> writes:
Walter Bright wrote:

 Walter Bright wrote:

 Brad Roberts wrote:

 On Mon, 13 Aug 2007, Walter Bright wrote:

 I just don't see the issue.


 Apply this same logic to variable shadowing.  Shadowing has been 
 declared illegal.


 While pedantically it is the same logic, but practically I think it's 
 very different. I don't see writing derived classes like one writes a 
 nested scope.


 
 I want to add to that. For a nested scope, there really isn't any 
 barrier to renaming something. For a derived class, however, you have 
 users of it, and you may need to specifically hide a base member.


For that you can re-use override, eg.

class A
{
   int x;
}

class B
{
   override int x;
}

Regan
Aug 15 2007
prev sibling parent Bill Baxter <dnewsgroup billbaxter.com> writes:
Walter Bright wrote:

 Chris Nicholson-Sauls wrote:

 Or, as has happened to me a time or two, the programmer may not even 
 /know/ what's in the super class, if deriving from a class provided by 
 a library for example.  (This is particularly true with private 
 fields, of course.)


 
 Exactly what problem does this solve? If you're writing a derived class, 
 and you create another field of the same name as a field in the base 
 class, what bug has been created? The base class functions will still 
 use the base class field, the derived class functions will still use the 
 derived class field. The fields aren't polymorphic, so no hijacking can 
 happen.
 
 I just don't see the issue.
 


I think there is another interaction that needs to be considered, and 
that is what happens when simple fields are converted to property 
methods.  Ideally one should be able to change a field to a property at 
will with little or no repercussions.  Currently when you make *both* 
base and derived fields into properties suddenly the behavior changes 
from non-virtual to virtual.  Maybe it's a separate issue but it seems 
like this could also be a source of difficult to find bugs.   And it 
also seems like making overriding fields an error would fix it.



--bb
Aug 13 2007