[Issue 3581] "private" attribute breaks "override"
https://issues.dlang.org/show_bug.cgi?id=3581 Mathias LANG changed: What|Removed |Added Status|REOPENED|RESOLVED CC||pro.mathias.l...@gmail.com Resolution|--- |WORKSFORME --- Comment #21 from Mathias LANG --- D1 only, closing --
[Issue 3581] private attribute breaks override
http://d.puremagic.com/issues/show_bug.cgi?id=3581 yebblies yebbl...@gmail.com changed: What|Removed |Added Version|1.051 |D1 AssignedTo|nob...@puremagic.com|bugzi...@digitalmars.com --- Comment #19 from yebblies yebbl...@gmail.com 2012-01-30 16:52:50 EST --- Fixed for D2 only: https://github.com/D-Programming-Language/dmd/commit/07fccae099cf93bffd5f5459e43a1e908213d7f4 Assigning to Walter for D1 merge or closing. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 3581] private attribute breaks override
http://d.puremagic.com/issues/show_bug.cgi?id=3581 yebblies yebbl...@gmail.com changed: What|Removed |Added CC||rayerd@gmail.com --- Comment #20 from yebblies yebbl...@gmail.com 2012-01-30 16:53:58 EST --- *** Issue 4855 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: ---
[Issue 3581] private attribute breaks override
http://d.puremagic.com/issues/show_bug.cgi?id=3581 yebblies yebbl...@gmail.com changed: What|Removed |Added CC||yebbl...@gmail.com --- Comment #16 from yebblies yebbl...@gmail.com 2011-07-08 16:47:50 EST --- I've submitted the phobos patch as: https://github.com/D-Programming-Language/phobos/pull/137 -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 3581] private attribute breaks override
http://d.puremagic.com/issues/show_bug.cgi?id=3581 yebblies yebbl...@gmail.com changed: What|Removed |Added CC||blazej.podsia...@gmail.com --- Comment #17 from yebblies yebbl...@gmail.com 2011-07-08 17:00:15 EST --- *** Issue 6266 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: ---
[Issue 3581] private attribute breaks override
http://d.puremagic.com/issues/show_bug.cgi?id=3581 --- Comment #15 from Rainer Schuetze r.sagita...@gmx.de 2011-05-06 23:37:50 PDT --- (In reply to comment #13) See proposed patch and problems with patch: https://github.com/donc/dmd/commit/9f7b2f8cfe5d7482f2de7f9678c176d54abe237f#commitcomment-321724 Copying my comment on github for better visibility: As I happen to have the patch installed, I stumbled over this problem today when running the unittests. The problem is that the override sets the attribute for the complete mixin, including auto-implemented constructors. Here's a patch that moves the override attribute to each generated function if it is not a constructor: diff --git a/std/typecons.d b/std/typecons.d index e0868b0..979b1d1 100644 --- a/std/typecons.d +++ b/std/typecons.d @@ -1593,7 +1593,7 @@ class AutoImplement(Base, alias how, alias what = isAbstractFunction) : Base private alias AutoImplement_Helper!( autoImplement_helper_, Base, Base, how, what ) autoImplement_helper_; -override mixin(autoImplement_helper_.code); +mixin(autoImplement_helper_.code); } /* @@ -2081,6 +2081,8 @@ private static: enum storageClass = make_storageClass(); // +if(isAbstractFunction!func) +code ~= override ; code ~= Format!(extern(%s) %s %s(%s) %s %s\n, functionLinkage!(func), returnType, -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 3581] private attribute breaks override
http://d.puremagic.com/issues/show_bug.cgi?id=3581 --- Comment #14 from Don clugd...@yahoo.com.au 2011-03-30 04:39:53 PDT --- The failure is probably related to bug 2525 and 3525: functions in interfaces aren't dealt with correctly. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 3581] private attribute breaks override
http://d.puremagic.com/issues/show_bug.cgi?id=3581 Walter Bright bugzi...@digitalmars.com changed: What|Removed |Added CC||bugzi...@digitalmars.com --- Comment #13 from Walter Bright bugzi...@digitalmars.com 2011-03-29 14:59:55 PDT --- See proposed patch and problems with patch: https://github.com/donc/dmd/commit/9f7b2f8cfe5d7482f2de7f9678c176d54abe237f#commitcomment-321724 -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 3581] private attribute breaks override
http://d.puremagic.com/issues/show_bug.cgi?id=3581 Jonathan M Davis jmdavisp...@gmx.com changed: What|Removed |Added CC||jmdavisp...@gmx.com --- Comment #4 from Jonathan M Davis jmdavisp...@gmx.com 2011-03-01 00:08:33 PST --- See bug# 4542. This issue is a highly debated one. TDPL says that you should be able to override private (primarily with the idea of using NVI). dmd is not currently implemented that way - private functions are _never_ virtual. NVI is highly useful, but it can be essentially done with protected instead of private, and if you make it so that you can override private methods, then you can't inline them anymore unless you declare them final. So, you can currently do NVI - you just have to use protected instead of private - and you get the higher efficiency of private functions being inlineable, but dmd is not in line with TDPL. On the other hand, if you make dmd in line with TDPL, then you can use private for NVI (which is of some benefit conceptually at least - though not really pratically-speaking), but then private functions are no longer inlineable, which could be a definite performance hit. I'm not sure that it has been definitely decided that dmd will be made to match TDPL in this case or if TDPL will have to be changed. Walter has never said anything on the matter as far as I recall, and I don't remember what Andrei said (if anything) the last time it was discussed. At this point, I'm inclined to say that TDPL should be changed, since I don't like the idea of losing out on inlineable private functions without having to mark them all as final, but I don't know what the plan is at this point (if there even is one). I think that Andrei assumed that private was overridable, because it is because it is in C++ (though C++ doesn't force everything to be virtual like D does, so the cost there isn't the same) as oppose to having discussed it with Walter, but I don't know. So, it's clear that per TDPL, private should be overridable, but I don't think that it's clear that TDPL is going to win out on this one. Walter (and possibly Andrei) need(s) to make a decision here, and I don't know if they've even discussed it. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 3581] private attribute breaks override
http://d.puremagic.com/issues/show_bug.cgi?id=3581 Rainer Schuetze r.sagita...@gmx.de changed: What|Removed |Added CC||r.sagita...@gmx.de --- Comment #5 from Rainer Schuetze r.sagita...@gmx.de 2011-03-01 12:36:28 PST --- I think it is often forgotten that protection flags in D are different from C. For example, private does not restrict access to the class, but to the module, so you can inherit from the class and still have full access to the private member functions of the base class. That means it makes sense to allow a private function to be virtual. I'd also say that protection attributes should not interfere with attributes that deal with virtuality (abstract, override, final) - they are othogonal. So I would vote for TDPL. Note that the compiler has access to all overrides of the private functions (they must be in the same module), so it might still inline them if never overridden. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 3581] private attribute breaks override
http://d.puremagic.com/issues/show_bug.cgi?id=3581 --- Comment #6 from Rainer Schuetze r.sagita...@gmx.de 2011-03-01 12:49:09 PST --- Thinking about it again, I might have missed the point completely. I don't have TDPL handy right now, but IIRC it allowed overriding the private functions in another mode, doesn't it? That would make the inlining of private members impossible unless final is specified. I still think the two concepts protection/virtuality should be kept apart. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 3581] private attribute breaks override
http://d.puremagic.com/issues/show_bug.cgi?id=3581 --- Comment #7 from Jonathan M Davis jmdavisp...@gmx.com 2011-03-01 14:48:17 PST --- They can't be kept apart as long as you can't specify whether a function is virtual or not. In C++, a member function is not virtual unless you mark it as virtual (or a base class has a function with the same name which is marked virtual). So, you can get into problems where you override functions but don't make them virtual, so which version of a function you call depends on what type of pointer you're using (yuck). D just makes all member functions virtual except those which it knows don't have to be. It currently treats private as non-virtual (and therefore non-overridable since all overridable functions must be virtual in D as just mentioned), presumably with the idea that there's no point in overriding a private function (forgetting NVI). C++ does allow the overiding of private functions. The typical case where you do that would be NVI, where you make a public function which is non-virtual and a private one which is virtual and overridden in derived classes. That way, the private, virtual function can only be called in the base class, and you can enforce that certain things happen before or after the call to the private function, because the only place that it ever gets called is within the public, non-virtual function in the base class. It's a nice idiom, but it can be pretty much done the same way using protected instead of private (particularly since there are ways to get around the uncallability, IIRC - probably by just casting the this pointer to the base class type). It also doesn't cause a penalty in C++, because your private functions are usually non-virtual and completely inlinable. In D, however, because you _can't_ specify a function as non-virtual, if you made private functions overridable, they must be virtual, and all of a sudden, the compiler can't inline _any_ private functions unless they're final. It doesn't know what all of the derived classes look like and whether they override a particular function, so it must assume that they'll be overriden and would have to make the virtual. So, if we make it possible to override private functions in D, it would pretty much have to become common practice to mark all private functions as final unless you were specifically using NVI, otherwise you're going to take a definite performance hit in a lot of cases. On the other hand, we could leave private as unoverridabel and just say that NVI has to use protected instead of private. It works just as well and doesn't require that you mark almost every private function that you ever write as final. You _can't_ separate access level from virtuality in D, because non-virtuality is done as an optimization rather than a function being virtual on non-virtual being at the programmer's request. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 3581] private attribute breaks override
http://d.puremagic.com/issues/show_bug.cgi?id=3581 --- Comment #8 from Vladimir thecybersha...@gmail.com 2011-03-01 15:37:06 PST --- (In reply to comment #7) They can't be kept apart as long as you can't specify whether a function is virtual or not. What do you mean? Isn't that exactly what final does? Up until the moment you mention final, your message reads as if it was written by someone who didn't know final existed. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 3581] private attribute breaks override
http://d.puremagic.com/issues/show_bug.cgi?id=3581 --- Comment #9 from Jonathan M Davis jmdavisp...@gmx.com 2011-03-01 16:08:37 PST --- Final says that a function can't be overridden. That doesn't necessarily make it non-virtual. For instance, if that function is already overriding another function, then final isn't going to make it non-virtual. It does give you some indirect control over whether a function is virtual or not, but it's not quite the same thing. Really, final is intended for preventing overiding, not for making a function non-virtual. It just does that as an optimization, because it knows that it can. You don't explicitly have control over whether a function is virtual or not. Regardless, the fact that the only way to make a function non-virtual (assuming that private is overridable) is if you specifically mark it is a final means that most private functions _will_ be virtual and will _not_ be inlinable, because the average programmer won't realize that they have to do that to make the function inlinable. So, it's going to be a definite performance hit if private becomes overridable. It makes the default inefficient for essentially no gain. You can still do NVI with protected, so the only real reason IMO to make private overridable is simply because TDPL said that you could in its discussion of NVI. I think that it would be better in this case to fix TDPL than change dmd. Otherwise, the cost in efficiency is too high for essentially no gain. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 3581] private attribute breaks override
http://d.puremagic.com/issues/show_bug.cgi?id=3581 --- Comment #10 from Vladimir thecybersha...@gmail.com 2011-03-01 16:25:18 PST --- Can protected replace private in all intended usage of virtual private functions? If so, has it been considered to simply forbid virtual private functions (with a warning or error)? Then programmers should clarify whether they need NVI, or a non-virtual, inlinable private function (by adding final). -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 3581] private attribute breaks override
http://d.puremagic.com/issues/show_bug.cgi?id=3581 --- Comment #11 from Don clugd...@yahoo.com.au 2011-03-01 16:56:18 PST --- Regardless of this, the patch is still valid (if a function isn't virtual, you shouldn't be allowed to mark it as override). The question of whether private means virtual is an independent issue. Could someone open a new bug for that please? -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 3581] private attribute breaks override
http://d.puremagic.com/issues/show_bug.cgi?id=3581 --- Comment #12 from Jonathan M Davis jmdavisp...@gmx.com 2011-03-01 17:35:58 PST --- True. The problem is that dmd doesn't generally complain about invalid attributes. And so this case is included. The patch does fix this one instance of invalid attributes being allowed (and ignored). Bug# 4542 is essentially a bug on whether private functions should be overridable or not. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 3581] private attribute breaks override
http://d.puremagic.com/issues/show_bug.cgi?id=3581 Don clugd...@yahoo.com.au changed: What|Removed |Added Keywords|wrong-code | CC||clugd...@yahoo.com.au --- Comment #2 from Don clugd...@yahoo.com.au 2011-02-28 14:22:18 PST --- Applies to 'static override' as well. One possible patch is func.c, FuncDeclaration::semantic(), line 400: // if static function, do not put in vtbl[] if (!isVirtual()) { //printf(\tnot virtual\n); +if (isOverride()) +error(cannot use override with non-virtual functions); goto Ldone; } But this should really be caught in the parser, I think. Anyway, it's accepts-invalid rather than wrong-code. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 3581] private attribute breaks override
http://d.puremagic.com/issues/show_bug.cgi?id=3581 Jacob Carlborg d...@me.com changed: What|Removed |Added CC||d...@me.com --- Comment #3 from Jacob Carlborg d...@me.com 2011-02-28 23:24:44 PST --- In TDPL, page 214-215, we can see examples of methods declared as private and override. In TDPL private methods are virtual, in DMD they're non-virtual. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 3581] private attribute breaks override
http://d.puremagic.com/issues/show_bug.cgi?id=3581 nfx...@gmail.com changed: What|Removed |Added Status|NEW |RESOLVED Resolution||WONTFIX -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 3581] private attribute breaks override
http://d.puremagic.com/issues/show_bug.cgi?id=3581 Brad Roberts bra...@puremagic.com changed: What|Removed |Added Status|RESOLVED|REOPENED CC||bra...@puremagic.com Resolution|WONTFIX | -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 3581] private attribute breaks override
http://d.puremagic.com/issues/show_bug.cgi?id=3581 Vladimir thecybersha...@gmail.com changed: What|Removed |Added CC||thecybersha...@gmail.com --- Comment #1 from Vladimir thecybersha...@gmail.com 2010-03-09 14:33:06 PST --- Simpler test case: class C { private override void f() {} } This code should not compile. Removing private produces the expected function test.C.f does not override any function error. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---