www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 5056] New: Warning against virtual method call from constructor

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5056

           Summary: Warning against virtual method call from constructor
           Product: D
           Version: D1 & D2
          Platform: Other
        OS/Version: Windows
            Status: NEW
          Keywords: diagnostic
          Severity: minor
          Priority: P2
         Component: DMD
        AssignedTo: nobody puremagic.com
        ReportedBy: bearophile_hugs eml.cc



I suggest to add a compiler suggestion/tip (a kind of warning) against
(transitively) calling virtual methods inside its constructors, because doing
so causes the most-derived override to be called, regardless of whether the
constructor for the type that defines the most-derived override has been
called.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 15 2010
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5056


Jonathan M Davis <jmdavisProg gmx.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jmdavisProg gmx.com



PDT ---
It should be noted that unlike C++ this isn't necessarily a problem in D. In
C++, each class in the class's hierarchy is constructed one by one until the
most derived class constructor is called. So, if you call a virtual function in
a constructor, the object is not in a safe state and won't even correctly know
what type it is.

In D, however, the object is in a safe state. All of the classes in its
hierarchy are properly put together with the default values for all their
variables already initialiazed, _then_ the constructors are called. So, virtual
functions will work correctly. It is perfectly safe to call them.

Now, it's quite possible that you write your class poorly and a virtual
function that you call in your constructor assumes that the super classes'
constructors have already been called, and it doesn't do what you expect, but
it's nowhere near the same problem as it is in C++. And since, unlike C++,
nearly all member functions in D are virtual, you have somewhat less control
over whether a function is or isn't virtual, and it will be that much harder to
guarantee that a function isn't virtual. You'd be restricted to private
functions and public final functions which don't override base class functions.
In many cases, that is unnecessarily restrictive.

So, while this is definitely a concern in C++, I'm not sure that there's much
pont in worrying about it in D. D has specifically been designed to avoid the
worst problems associated with calling virtual functions in a constructor.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 15 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5056




07:36:52 PDT ---
I'd say, this can be more serious issue with D than with C++ because D's
execution is repeatable, so if you have a security issue, it becomes
deterministic. Just a thought: if you have a password field, by default it's
initialized to null, which can cause unauthorized access.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 16 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5056


Steven Schveighoffer <schveiguy yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |schveiguy yahoo.com



09:50:22 PDT ---

 It should be noted that unlike C++ this isn't necessarily a problem in D. In
 C++, each class in the class's hierarchy is constructed one by one until the
 most derived class constructor is called. So, if you call a virtual function in
 a constructor, the object is not in a safe state and won't even correctly know
 what type it is.
In C++, calling a virtual function from a constructor calls that class' version of the virtual function directly. In other words, virtual functions are not virtual inside the constructor. In D, the virtual function is called, so it could potentially be a problem. However, it's also possible in D (unlike in C++) to initialize data before calling the parent constructor through the use of super. But of course, the base class has no say in whether the derived class's version is safe to call. So yes, I think it would be a good thing to suggest as a "best practice" (certainly not a compiler error or warning), but there are ways to make it safe. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Oct 16 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5056


nfxjfg gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nfxjfg gmail.com



The really bad thing is that D doesn't allow you to set up a sub class before
calling the super constructor. Consider this:

class Foo : Moo {
  int stuff;
  this(int stuff_) {
    this.stuff = stuff_;
    super();
  }
  override int get_stuff() {
     return stuff;
  }
}

This won't compile, because the compiler forces you to do the super ctor call
_before_ setting up any members. This was probably thought as a feature to
ensure "correct" construction, but it just doesn't work because the super ctor
could call virtual functions. And it's really hard to work this around.

This behaviour of super ctor calls and virtual functions has been badly
thought-through and doesn't increase productivity in the slightest. Sucks a
lot.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 16 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5056




10:43:10 PDT ---

 The really bad thing is that D doesn't allow you to set up a sub class before
 calling the super constructor. Consider this:
 
 class Foo : Moo {
   int stuff;
   this(int stuff_) {
     this.stuff = stuff_;
     super();
   }
   override int get_stuff() {
      return stuff;
   }
 }
 
 This won't compile, because the compiler forces you to do the super ctor call
 _before_ setting up any members. This was probably thought as a feature to
 ensure "correct" construction, but it just doesn't work because the super ctor
 could call virtual functions. And it's really hard to work this around.
Hm... I just tried it, it does work. Were you thinking of something else? example: import std.stdio; class A { this() { foo(1); } void foo(int x) { writeln("A.foo"); } } class B : A { int x; this() { writeln("B.this"); x = 5; super(); } override void foo(int x) { writeln("B: ", this.x); } } void main() { auto b = new B; } prints: B.this B: 5 -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Oct 16 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5056





 Hm... I just tried it, it does work.  Were you thinking of something else?
This confuses me. It works both in D1 and D2. Maybe it's a regression? The spec says: "3. It is illegal to refer to this implicitly or explicitly prior to making a constructor call." http://www.digitalmars.com/d/1.0/class.html#constructors And I thought past versions of dmd conform to this, but apparently I'm wrong. writefln in D1), I retract my statement and apologize for the noise. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Oct 16 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5056




12:09:12 PDT ---


 Hm... I just tried it, it does work.  Were you thinking of something else?
This confuses me. It works both in D1 and D2. Maybe it's a regression? The spec says: "3. It is illegal to refer to this implicitly or explicitly prior to making a constructor call." http://www.digitalmars.com/d/1.0/class.html#constructors
I interpret that the same way you do. I think maybe that this is a rule that was removed, but someone forgot to remove it from the docs? I think you should file a separate doc bug on it. It doesn't strike me as a good requirement anyways. What problems does having that restriction prevent?
 And I thought past versions of dmd conform to this, but apparently I'm wrong.
BTW, the file compiled with the oldest compiler I had installed (2.033). So it certainly has been this way for some time. I expect at some point it was as the docs state or else why would that rule even be there? -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Oct 16 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5056




12:26:11 PDT ---
see bug 3393

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 16 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5056




Thank you for all the comments.


 So yes, I think it would be a good thing to suggest as a "best practice" 
 (certainly not a compiler error or warning), but there are ways to make it
 safe.
It may be a third class of compiler messages, beside the errors and the warnings. The compiler "tips" may warn against possible troubles like this one (also see bug 4924 and others), give suggestions for improvements or, like the CommonLisp compilers, warn against not efficient code. An example of those comments from a CLisp compiler: http://shootout.alioth.debian.org/u32/program.php?test=mandelbrot&lang=sbcl&id=1 -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Oct 17 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5056


Witold Baryluk <baryluk smp.if.uj.edu.pl> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |baryluk smp.if.uj.edu.pl



19:07:23 PST ---
 This confuses me. It works both in D1 and D2. Maybe it's a regression?
 The spec says:
 
 "3. It is illegal to refer to this implicitly or explicitly prior to making a
 constructor call."
 http://www.digitalmars.com/d/1.0/class.html#constructors
I interpret that the same way you do. I think maybe that this is a rule that was removed, but someone forgot to remove it from the docs? I think you should file a separate doc bug on it. It doesn't strike me as a good requirement anyways. What problems does having that restriction prevent?
Hmm. I was using this "feature", setting fields before calling super(), for VERY long time. It worked, so i never actually payed attention, if i call super() at the beginning or the end of constructor - both worked well. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Nov 21 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5056


Michal Minich <michal.minich gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |michal.minich gmail.com



07:26:54 PDT ---
dmd 2.063 - instance method is called before constructor.

module test;
import std.stdio;

class Base
{
    this () {
       writeln("Base.this"); 
       foo(); // here should come warning: calling virtual method in
constructor
    }

    void foo () { writeln("Base.foo"); }
}

class Derived : Base
{
    this () { writeln("Derived.this"); }
    override void foo () { writeln("Derifed.foo"); }
}

void main () { auto d = new Derived; }

Program output:
Base.this
Derifed.foo    // method Derived.foo is called before object constructor
Derived.this

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 05 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5056


Andrej Mitrovic <andrej.mitrovich gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andrej.mitrovich gmail.com



07:53:35 PDT ---

 dmd 2.063 - instance method is called before constructor.
There's a simple fix for this, simply call super() directly whenever you want to: class Derived : Base { this() { writeln("Derived.this"); super(); } override void foo () { writeln("Derifed.foo"); } } Result: Derived.this Base.this Derifed.foo -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Jun 05 2013
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=5056




08:16:45 PDT ---


 dmd 2.063 - instance method is called before constructor.
There's a simple fix for this, simply call super() directly whenever you want
Slightly better, but hardly a fix. at best partial workaround. you exchange one invalid initialization order for other invalid order - now Derived is initialized before Base. I guess is a less of a problem as developer of Derived can handle this invalid order locally. Think of Base and Derived to be developed by independent developers not knowing the details of implementation: In current scheme, to ensure that derived class is initialized before any overridden method can be called: developer has to explicitly call the super() constructor as the last step in derived constructor. And that must be done for every derived class on which any method is overridden. Also this would not only for the most-derived class, if there is more derived classes in hierarchy - every one hast to do it. Two sides of problem are: - Developer of Base might not be aware that method called in constructor is virtual, so he accidentally allows it to be hijacked. - Developer of Derived might be not aware that method he has overridden is called in Base constructor, which makes his overridden method to be allays called before his constructor his called. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Jun 05 2013