search

sociomantic-tsunami / ocean

General purpose, platform-dependent, high-performance library for D
Other
61 stars 56 forks source link

Make deprecated D1-style operator overloads `final`. #794

Closed don-clugston-sociomantic closed 4 years ago

don-clugston-sociomantic commented 4 years ago

From DMD 2.089, D1-style operator overloads are deprecated. This may be a problem because the D2-style operator overloads don't support virtual functions. There is no equivalent functionality.

On the D newsgroup it was stated that virtual overloaded operators is an extremely rare, bizarre case. I think that's true for normal operators like +, -, *, /. However D also uses the same syntax for the non-mathematical operator in. It makes perfect sense for this to be an overridden virtual function. eg value in Container may be overridden to value in SinglyLinkedList, value in HashMap, etc. Any object-oriented container library would do this, I think.

It turns out we do indeed have cases in ocean which are overloading operators virtually.

BUT maybe they don't actually need to be virtual. Maybe we're not relying on that.

This PR deals with the hard cases. It takes all of the operator overloads which are virtual functions, and makes them non-virtual. This will show us how much code will be broken as a result of this. There was one case in the module Enum which declared a virtual opIn, and then overrode it. I took more drastic action in that case: I removed the virtual declaration, and was able to make override function into a static function.

I don't know if this will really work in all cases, but I tested a couple of my apps, and they were not affected by this change.

This builds upon my previous PR, but it's actually independent from it, I think (I just did it this way so I could be sure I'd caught all the relevant functions).

codecov[bot] commented 4 years ago

Codecov Report

Merging #794 into v5.x.x will decrease coverage by 0.05%. The diff coverage is 100%.

alaksiej-stankievicx-dunnhumby commented 4 years ago

Do I understand correctly that I should try to build all my application with these changes, to verify we have no need in virtual behaviour?

don-clugston-sociomantic commented 4 years ago

Yes, it would be great if you can test this. I think your apps are the ones most likely to have problems.

don-clugston-sociomantic commented 4 years ago

With the fix which has been merged into upstream dmd, there is now a better way to fix this. We can introduce new D2 functions which are aliased to the D1 functions. This will allow us to fix the deprecations without breaking any code.