Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[do not merge] trigger deprecation warnings for auto-decoding #4384

Closed
wants to merge 1 commit into from

Conversation

wilzbach
Copy link
Member

@wilzbach wilzbach commented May 30, 2016

Just a proof of concept PR that shows how auto-decoding could be deprecated.
Same goes for the deprecation message.

I added deprecation warning for narrow strings their range primitives (front, popFront, back and popBack) based on @adamdruppe's proposal.

We only have to fix 132 locations

Happy fixing!

@tsbockman
Copy link
Contributor

tsbockman commented May 31, 2016

I, personally, support this approach, but I think @andralex is set to veto it:

I'm not going to debate this further (though I'll look for meaningful answers to the questions above). But this thread has been informative in that it did little to change my conviction that autodecoding is a good thing for D, all things considered (i.e. the wrong decision to not encapsulate string as a separate type distinct from bare array of code units). I'd lie if I said it did nothing. It did, but only a little.

Funny thing is that's not even what's important. What's important is that autodecoding is here to stay - there's no realistic way to eliminate it from D. So the focus should be making autodecoding the best it could ever be.

I'm not sure why he says "there's no realistic way to eliminate it from D": supposing that we all agreed that auto-decoding is a bad thing, is there any reason we can't just permanently deprecate it - but never actually remove it?

I think that would neatly solve any concerns about 10-year-old code silently changing in behaviour down the line, and it's not like we need to free up the deprecated template signatures for some other use.

@adamdruppe
Copy link
Contributor

On Mon, May 30, 2016 at 06:23:23PM -0700, tsbockman wrote:

I, personally, support this approach, but I think @andralex is set to veto it:

Andrei is wrong. We can all see it, and maybe if we demonstrate that a migration path is possible, even actually pretty easy following a simple deprecation path, maybe he can see it too.

@@ -2057,7 +2057,12 @@ version(unittest)
static assert(!is(typeof({ void[] a; popFront(a); })));
}

enum stringDeprecationMsg = "Unicode string require explicit iteration." ~
Copy link
Contributor

@tsbockman tsbockman May 31, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"string require" should be "strings require" or "string requires"
"explicit iteration." should be " the range element type to be specified explicitly: "

@andralex
Copy link
Member

@wilzbach thanks for running this experiment.

Andrei is wrong.

Definitely wouldn't be the first time and not the last.

We can all see it, and maybe if we demonstrate that a migration path is possible, even actually pretty easy following a simple deprecation path, maybe he can see it too.

I'm not sure who "all" is but that's beside the point. Taking a step back, we'd take in a change that breaks Phobos in 132 places only if it was a major language overhaul bringing dramatic improvements to the quality of life for D programmers. An artifact as earth shattering as ranges, or an ownership system that was massively simple and beneficial. For comparison, the recent changes in name lookup broke Phobos in fewer places (I don't have an exact number, but I think they were at most a couple dozen.) Those changes were closing an enormous hole in the language and mark a huge step forward. I'd be really hard pressed to characterize the elimination of autodecoding as enough of an improvement to warrant this kind of breakage. (I do realize there's a difference between breakage and deprecation, but for many folks the distinction is academic.)

The better end game here is to improve efficiency of code that uses autodecoding (e.g. per the recent find() work), and to make sure RCStr is the right design. A string that manages its own memory and does the right things with regard to Unicode is the ticket. Let's focus future efforts on that.

@andralex andralex closed this May 31, 2016
@andralex
Copy link
Member

One more thing: a possible additional outcome of this experiment would be to append .representation to select places in Phobos that would work fine with and without autodecoding.

@tsbockman
Copy link
Contributor

I'm not sure who "all" is but that's beside the point.

That would be just about every single person who participated in the recent forum thread, except you - including a goodly percentage of all currently active D contributors.

a change that breaks Phobos in 132 places

Phobos is naturally more effected by this change than typical user code will be, because Phobos exists to abstract over a lot of the kind of low-level text operations that might need to work with code units. Also, 132 really isn't much at all considering how trivial the fix is, and how large Phobos is.

(I do realize there's a difference between breakage and deprecation, but for many folks the distinction is academic.)

Indeed, this change breaks nothing; it is effectively just a way of warning the user that his code might not be doing what he thinks it does.

only if it was a major language overhaul bringing dramatic improvements to the quality of life for D programmers.

This change would:

  1. Put an end to the need to add a special case to much generic range code, everywhere, just to work around this unwanted mis-feature.
  2. Reduce D's need for people like Scott Meyers - that is to say, removing auto-decoding would make the language easier to understand, less complex, and less surprising to newcomers.

@CyberShadow
Copy link
Member

I think # of breakages in Phobos is a poor metric of how much we will break user code, for the simple reason that Phobos contains the implementation of autodecoding. A more interesting (and accurate) test would be to test dub packages on code.dlang.org, for example.

@CyberShadow
Copy link
Member

I understand that this PR's test results can be used to track Phobos' reliance on auto decoding, which I understand we've agreed is a non-controversial first step away from auto-decoding. Should I reopen it for that purpose, then?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants