Revert "add unsetindex support to more copyto methods (#51760)"

[KristofferC]

This reverts commit f0a28e9.

This introduced in general a try catch inside the inner loop for copyto! and it also has performance regression in other cases #53430.

Since this was added without any tests and "is not-quite-public API" it seems easiest to just revert it.
This was added for Memory-to-Array and vice versa but dedicated methods could be added for that if it is desirable

Fixes #53430, #52070

[vtjnash]

I think you must have this for Memory-to-Array and Array-to-Memory to work correctly. Therefore, shouldn't this only revert on the release backports branch target?

[KristofferC]

Therefore, shouldn't this only revert on the release backports branch target?

The regressions are there on master as well. The original PR added no tests and seemed to be more of an off-hand thing. If this PR starts failing some tests it should be possible to add it back restricted to Memory + Array.

[vtjnash]

It looks like this revert will re-open #45125

[KristofferC]

Also fixes #52070

[KristofferC]

It looks like this revert will re-open #45125

That's not a problem because it is already open 🤣

[KristofferC]

Also, the triage opinion there seems to have been (quoting Jeff):

I believe this was added for a use case in the compiler. I would prefer the compiler to use some different approach if possible, and roll this back, since it is unnerving for it to be inconsistent like this.

Which seems to indicate that there should be less of this, not more.

[vtjnash]

Well, it is at least less inconsistent currently on master

[jishnub]

#53383 may need to be reverted as well along with this, as it added tests for the copyto! branches

[KristofferC]

I reverted that as well. I don't think that it is at all established that the #undef "support" should be extended everywhere. The position (at least how I have understood it) is that once something gets defined it should not revert to undefined (#54332 (comment)).

[vtjnash]

That is a bit of a misconception, as that position only applies to struct fields and not arbitrary computations (such as getindex). We rely heavily on it being possible for other operations (on Arrays) to become undef, and nothing does or could restrict that without being a major breaking change

[KristofferC]

Yes, as said in #45125 (comment) it might be better for the compiler to use some other method for this than copyto!.

[vtjnash]

I suppose, but I hate the idea of having 2 functions, where the only difference is that one is broken and the other works, where we revert the working one to the old broken one, then introduce a copy of that function that works correctly

[KristofferC]

@jishnub, this required more reverts than ideal since some of your PRs built on this and it is hard for me to do things more fine grained than full revert of commits. Maybe you have a better idea of how to do this...

[jishnub]

I don't have a better idea either. I think this is fine, as the unrelated features in those PRs shouldn't be hard to re-land.

[KristofferC]

Alright, thanks. I'm happy that relanding those features wouldn't be too bad.