Revert "Fix Issue 18665 - Deprecate Undocumented Operator Overloads"

[WalterBright]

Reverts #10130

This is causing massive, expensive problems for one of our users.

[dlang-bot]

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	18665	enhancement	Deprecate Undocumented Operator Overloads

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#10716"

[Geod24]

You're talking about https://forum.dlang.org/thread/erktpojjcjmzvrbnxkpw@forum.dlang.org right ?

I think @don-clugston-sociomantic 's point here is that, while deprecating opAdd and opSub makes sense, opIn & opCat don't make so much sense as they tend not to share code (like add, sub, div, mul, etc... would). Additionally, the way operator overload interact with classes can indeed be surprising (and forces some boilerplate). @don-clugston-sociomantic correct me if I misunderstood.

The D1 operator overloads were deprecated for a reason, namely a bug found when they were coupled with D2 operator overloads. Said bug can of course be fixed, but there's an inherent complexity coming from the ability to have two ways to do operator overloading.

I hope we can come up with better ways to deal with this issue than a flat out revert.
If the deprecation hadn't been released yet, reverting would be the best course of action, but this had been in for 3 releases already.

[WalterBright]

I hope we can come up with better ways to deal with this issue than a flat out revert.

So do I. But they need a fix now.

[thewilsonator]

I'm not sure that pushing this to stable is a good idea. If at all, it should be behind an opt-out switch since there is a very real reason for this being deprecated in the first place.

[WalterBright]

There's no urgent reason to deprecate it now, and they need a fix now.

[WalterBright]

I'm not sure how to rebase this to stable, as the instructions only apply if I have a local git version of the PR? Halp!

[thewilsonator]

Hardly, this has been deprecated for 6 months. That doesn't scream urgency.

[thewilsonator]

Should be behind an opt out switch, not reverted.

[WalterBright]

That doesn't scream urgency.

It's urgent because the user requires a major bug fix that was fixed in the newer compiler, but cannot upgrade because of this problem.

[thewilsonator]

Let me rephrase that: Why does -dw not fix this issue (e: assuming they are compiling with -de)? That is the entire point of that switch.

[don-clugston-sociomantic]

Hardly, this has been deprecated for 6 months. That doesn't scream urgency.

The idea that everyone is using the latest compiler release is quite wrong.
The urgency comes from a Ubuntu release going out of support. To move to a supported Ubuntu release we were forced to use the latest DMD. These two deprecations prevent us from doing that.
A lot of the code which is affected is not in active development.

I think @don-clugston-sociomantic 's point here is that, while deprecating opAdd and opSub makes sense, opIn & opCat don't make so much sense as they tend not to share code (like add, sub, div, mul, etc... would). Additionally, the way operator overload interact with classes can indeed be surprising (and forces some boilerplate). @don-clugston-sociomantic correct me if I misunderstood.

This is correct. Note also that opIndex remains virtual, while opIn is not. That is a bizarre inconsistency. And because this makes a change from virtual to non-virtual, the effects cross from library to user code.

To be clear: I think that deprecating opAdd and friends is basically a good idea. Arguably, the changes it makes it makes us make are good ones.

It's only opCat and opIn that are a problem.
Maybe it was a mistake to incorporate those two into opBinary and opBinaryRight. Personally I certainly don't think of in as a binary operator, I think of it as a form of indexing.

[thewilsonator]

These two deprecations prevent us from doing that.

I still don't see why -dw doesn't fix this problem.

[WalterBright]

Because that's a blunt instrument. They want to compile cleanly as a matter of policy (i.e. they like the other deprecations).

They simply need more time to deal with this. It is not a problem to revert this change for the time being.

[thewilsonator]

Because that's a blunt instrument.

That's literally the entire point of that switch, if it ain't sharp enough for this, why does it even exist?

It is not a problem to revert this change for the time being.

Then put this behind an opt out switch.

[Geod24]

To be clear: I think that deprecating opAdd and friends is basically a good idea. Arguably, the changes it makes it makes us make are good ones.

@don-clugston-sociomantic : From what I read, if the compiler was allowing you to reuse the name (opIn) without de deprecation message if a D2 operator is defined, would that solve your problem ?

[don-clugston-sociomantic]

@don-clugston-sociomantic : From what I read, if the compiler was allowing you to reuse the name (opIn) without de deprecation message if a D2 operator is defined, would that solve your problem ?

Oooh! That's a really interesting possibility.

[don-clugston-sociomantic]

Because that's a blunt instrument.

That's literally the entire point of that switch, if it ain't sharp enough for this, why does it even exist?

I find it useful as an exploratory tool. I use it while updating our libraries. But it's not suitable for use in production.

Note that we're mostly talking about libraries here. If you address it by a compiler switch it affects every library which that developer uses. That's not a solution.

[thewilsonator]

That was a mostly rhetorical question, but perhaps -de/-dw would benefit from selective like -revert/-transition?

[don-clugston-sociomantic]

Sorry, I did not intend to merge this. I think that the changes to the test suite should be reinstated. Only the deprecation messages should be removed, and even then, ideally only for classes, not for structs.

(don hides in shame)

[thewilsonator]

I was going to say, How did this merge with a blocking review?

[don-clugston-sociomantic]

Can you fix this up?

[don-clugston-sociomantic]

I'm surprised that the merge button isn't disabled, actually.