http://d.puremagic.com/issues/show_bug.cgi?id=3581



--- Comment #8 from Vladimir <thecybershadow@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: -------

http://d.puremagic.com/issues/show_bug.cgi?id=3581



--- Comment #9 from Jonathan M Davis <jmdavisProg@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: -------

http://d.puremagic.com/issues/show_bug.cgi?id=3581



--- Comment #10 from Vladimir <thecybershadow@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: -------

http://d.puremagic.com/issues/show_bug.cgi?id=3581



--- Comment #11 from Don <clugdbug@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: -------

http://d.puremagic.com/issues/show_bug.cgi?id=3581



--- Comment #12 from Jonathan M Davis <jmdavisProg@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: -------

http://d.puremagic.com/issues/show_bug.cgi?id=3581


Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugzilla@digitalmars.com


--- Comment #13 from Walter Bright <bugzilla@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: -------

http://d.puremagic.com/issues/show_bug.cgi?id=3581



--- Comment #14 from Don <clugdbug@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: -------

http://d.puremagic.com/issues/show_bug.cgi?id=3581



--- Comment #15 from Rainer Schuetze <r.sagitario@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: -------

http://d.puremagic.com/issues/show_bug.cgi?id=3581


yebblies <yebblies@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |yebblies@gmail.com


--- Comment #16 from yebblies <yebblies@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: -------

http://d.puremagic.com/issues/show_bug.cgi?id=3581


yebblies <yebblies@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |blazej.podsiadlo@gmail.com


--- Comment #17 from yebblies <yebblies@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: -------