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.


------- Comment #1 From Walter Bright 2007-08-13 13:34 [reply] -------

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.
 ------- Comment #1 From Walter Bright 2007-08-13 13:34 [reply] -------
 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:
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

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.
 ------- Comment #1 From Walter Bright 2007-08-13 13:34 [reply] -------
 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 next 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.
 ------- Comment #1 From Walter Bright 2007-08-13 13:34 [reply] -------
 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.

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.)

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 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
prev sibling parent 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