Bug 4542 – [tdpl] TDPL NVI example results in linker error

Trying an example from TDPL (p. 214) results in linker error. Source: import std.stdio; interface Transmogrifier { final void thereAndBack() { transmogrify(); untransmogrify(); } private: void transmogrify(); void untransmogrify(); } class CardBoardBox : Transmogrifier { override private void transmogrify() { writeln("transmogrify"); } override private void untransmogrify() { writeln("untransmogrify"); } } void main() { (new CardBoardBox).thereAndBack(); } Linker output: OPTLINK (R) for Win32 Release 8.00.2 Copyright (C) Digital Mars 1989-2009 All rights reserved. http://www.digitalmars.com/ctg/optlink.html impl.obj(impl) Error 42: Symbol Undefined _D4impl14Transmogrifier12transmogrifyMFZv impl.obj(impl) Error 42: Symbol Undefined _D4impl14Transmogrifier14untransmogrifyMFZv --- errorlevel 2 The same issue occurs, when private is replaced with package. protected and public do work.

This happens since private and package methods in D are implicitly final and thus cannot be overridden. I'm not sure whether it is a genuine bug in TDPL or just foreshadows to a planned change in dmd and the spec that hasn't been applied yet.

NVI is a highly useful idiom, and generally-speaking, dmd is supposed to come in line with TDPL rather than TDPL being in error. So, I'd definitely argue that the access level should have nothing to do with the overridability of a function, regardless of what was originally intended for D. We have final if we want to make functions non-overridable. There's no need to overload access level to make it do the same thing. I'd say that dmd and the spec should come in line with TDPL in this case.

Reassigning to Walter - this is a compiler issue.

Mass migration of bugs marked as x86-64 to just x86. The platform run on isn't what's relevant, it's if the app is a 32 or 64 bit app.

I'm going to chime in on this to say that I definitely recant what I said previously on this. I do _not_ think that TDPL is correct in this case. After having discussed it on the newsgroup previously, it was shown that you could do NVI with protected just fine and that using private for that doesn't actually buy you anything. However, if we make private overridable, then all of a sudden the default case for private is virtual and thus non-inlineable, which will definitely have a negative effect on performance. It _is_ possible to mark a private function as final to make it non-virtual and thus inlineable, but that means that if you want efficient private functions, you're going to have to get in the habit of specifically marking all of your private functions as final - just so that NVI can be done with private instead of protected in spite of the fact that protected does the job just fine. So, making private overridable makes the default case inefficient for essentially no gain.

(In reply to comment #5) > but that > means that if you want efficient private functions, you're going to have to get > in the habit of specifically marking all of your private functions as final Can't the compiler determine this automatically?

(In reply to comment 6) > Can't the compiler determine this automatically? No. In order to do that, it would have to know about every single class in the program that's derived from that class - directly or indirectly. That can't be known before linking. And by then, the function is already virtual or not. The compiler can only make a class' member function non-virtual when it can _know_ that no virtual calls to that function will ever be made. And that's can't generally be known. final guarantees it. I'm not sure that there's any other case when the compiler can determine it. Best case, there are some situations where it can know that a particular call doesn't need to be virtual. e.g. In (new A()).func() func could be done non-virtually by the compiler if it chose to optimize the code that way, because it can _know_ the exact type of A and avoid the virtual call (I don't know if it does). However, you can't even do that sort of optimization very often if you don't do flow analysis, which dmd typically avoids.

Well in any case, unless you interleave your private and public functions, you can use a shortcut: class Foo { final: private void x() {} private void y() {} } Has to be done for every class though, so maybe that's too much work..

Oh. It's totally feasible to mark all of a class' private member functions final. Personally, I always use private: already, so adding final: on top of that isn't exactly hard. However, the problem is that then everyone has to remember to do it. The default that people are generally going to want for private functions is that they be non-virtual, so if the default is virtual, that's a problem. Most private functions will then be virtual when they really should have been non-virtual, and it will cost performance - especially since you can't normally inline virtual functions. NVI is fairly rare, I think, and that's the only reason that I'm aware of that anyone would want to have a virtual private function. And since NVI can be done with protected anyway, I really don't think that it's much of a loss to make private always non-virtual as it is now. But TDPL says that private is virtual, so either we need to decide to update TDPL's errata accordingly or to change private to virtual to bring dmd in line with TDPL. Given the relatively low benefit for a fairly high cost, I really hope that we end up going against TDPL in this case. But it's obviously not my decision.

Isn't this what NVI is all about? import std.stdio; interface Foo { final void callable() { impl1(); impl2(); } void impl1(); void impl2(); } class Bar : Foo { @disable void impl1() { writeln("impl1"); } @disable void impl2() { writeln("impl2"); } } void main() { Foo foo = new Bar(); foo.callable(); } You can't directly call impl1() from within Bar, however you can call it via the Foo interface. I think this is pretty much it, no?

(In reply to comment #10) > however you can call it via > the Foo interface. * You can also call callable() via Bar, so this works fine: Bar bar = new Bar(); bar.callable();

The idea is to have a public, non-virtual function which does something before and/or after calling a private virtual function. That way, you can guarantee that certain things happen before or after the call to the private function. However IIRC, you can't actually make the private function non-callable. I forget what the details are. It was discussed in the newsgroup a while back, and Steven Schveighoffer was able to show that using private instead of public doesn't really buy you much in the way of protection - and the idiom works just fine when you use protected instead of private. It just means that there's no protection whatsoever against a derived class calling the virtual function directly. So, it's by convention at that point, but since you could get around it with private anyway (I don't remember exactly how at the moment, unfortunately), it private doesn't actually give you that guarantee anyway. So, NVI can be done with protected just fine. It might not be quite as nice that way, but it works. On the other hand, making private virtual will result in a systematic degredation in the performance of D programs in general. It can be overcome with final, but it mean that the default is worse performance. So, I really don't see much benefit in making private virtual. Protected will give you the main benefits of NVI just fine, whereas making private virtual gives you the slight benefit of increased cleanness in the use of NVI in some programs at the cost of performance in pretty much all programs.

What should have been done from day #1 is to implement the 'virtual' keyword and make methods non-virtual by default. It goes hand in hand with 'override'. I really don't know why virtual is on by default, maybe someone thought polymorphism would be used a lot in D but it turns out templates are much cooler to work with these days rather than OOP, just look at Phobos for example. (ok the last part is highly subjective :) ) (In reply to comment #12) > at the cost of performance in pretty much all programs. I wonder what would happen to performance if we suddenly switched behavior and made methods non-virtual by default (and require a 'virtual' keyword). Of course you'd have to fix your code and add 'virtual' to base methods, but this is practically an error-free refactoring since 'override' was already required. You would get CT errors rather than weird runtime behavior (like C++03).

(In reply to comment #13) > Of course you'd have to fix your code and add 'virtual' to base methods, but > this is practically an error-free refactoring since 'override' was already > required. You would get CT errors rather than weird runtime behavior (like > C++03). It's really not. You can have a class that someone else inherits from, which you don't control.

*** Issue 10422 has been marked as a duplicate of this issue. ***

This bug is still in dmd 2.071.2 and 2.072. I ran into this today, here's my reduced case: interface IA { package void f(); } void main() { IA a; a.f(); } Nothing new, but I'd like to nudge for resolution. :-) I dustmited my project due to this bug. The linker error didn't help me. If this is hard to fix, then maybe error out at compile-time as a hack? -- Simon

(In reply to Simon Naarmann from comment #16) > This bug is still in dmd 2.071.2 and 2.072. > > I ran into this today, here's my reduced case: > > interface IA { > package void f(); > } > void main() { > IA a; > a.f(); > } > > Nothing new, but I'd like to nudge for resolution. :-) I dustmited my > project due to this bug. The linker error didn't help me. > > If this is hard to fix, then maybe error out at compile-time as a hack? > > -- Simon I don't understand what you expect to happen in this case. You are calling f which does not have a body.

The spec seems to be clear about this [1]: """ The following are not virtual: - Struct and union member functions - final member functions - static member functions - Member functions which are private or package - Member template functions """ So it seems that TDPL is wrong in this aspect. I'm going to close this as per the arguments of Jonathan. [1] https://dlang.org/spec/function.html#virtual-functions

(In reply to RazvanN from comment #17) > (In reply to Simon Naarmann from comment #16) > > interface IA { > > package void f(); > > } > > void main() { > > IA a; > > a.f(); > > } > > I don't understand what you expect to happen in this case. You are calling f > which does not have a body. Indeed, if I don't inherit from IA, getting a linker error here is correct in this small example. My best guess of why I posted that reply 6 years ago: It was a faulty reduction of the original bug, but I mistook it for a correct reduction. I dustmited for the linker error and didn't pay attention to whether the reduced linker error arose with or without a class that tried to implement the private/package virtual function. I agree to treat this as a mistake in TDPL and to forbid overriding private/package methods, as the spec says. Nowadays, OP's example also produces a proper compiler error, much better than the original linker error: tdpl_private_virtual.d(18): Error: function `tdpl_private_virtual.CardBoardBox.transmogrify` `private` method is not virtual and cannot override tdpl_private_virtual.d(23): Error: function `tdpl_private_virtual.CardBoardBox.untransmogrify` `private` method is not virtual and cannot override With this good error already in place, I'm happy with the resolution as wontfix.