Skip to content

Fix Issue 18604 - in parameter storage class should be deprecated#8021

Closed
wilzbach wants to merge 1 commit intodlang:masterfrom
wilzbach:fix-18604
Closed

Fix Issue 18604 - in parameter storage class should be deprecated#8021
wilzbach wants to merge 1 commit intodlang:masterfrom
wilzbach:fix-18604

Conversation

@wilzbach
Copy link
Contributor

in currently just means const even though it was supposed to mean scope const. This makes it redundant and confusing and should be deprecated. Maybe after it has been deprecated and been an error for one version, it can be re-introduced with the proper meaning, but for now it should definitely trigger a deprecation warning.

dlang/druntime#2139 (comment)
https://dlang.org/spec/function.html#parameters


This change is rather trivial, but requires s/in /const / (or better s/in /scope const/) for druntime and Phobos.
As most of the work here is updating druntime, Phobos and the testsuite, I would only do so if there's a consensus that we can't fix in.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @wilzbach!

Bugzilla references

Auto-close Bugzilla Severity Description
18604 enhancement in parameter storage class should be deprecated

@mathias-lang-sociomantic
Copy link
Contributor

Oh god no. This will cause massive pain for user and bring very little benefit.
Also it's not equivalent to const ATM: https://issues.dlang.org/show_bug.cgi?id=17408

@jacob-carlborg
Copy link
Contributor

This just occurred to me: why is it required to create a DIP when adding new things to the language, but when removing it’s fine by just submitting a PR and hope someone will merge it?

@wilzbach
Copy link
Contributor Author

why is it required to create a DIP when adding new things to the language, but when removing it’s fine by just submitting a PR

Because DIPs are currently meant to die and have a very high chance of just going into oblivion without achieving anything. In the last ten months no DIP was neither approved nor rejected, in the meanwhile ~2500 PR have been merged.

hope someone will merge it?

I opened this to start a discussion. It would obviously require more work.
While we might not agree that that this is the best way forward, I think we can agree that in having lost its purpose sucks?

@ibuclaw
Copy link
Member

ibuclaw commented Mar 14, 2018

but when removing it’s fine by just submitting a PR and hope someone will merge it?

Most of the recent big deprecations/removals have been in the queue for years now.

While we might not agree that that this is the best way forward, I think we can agree that in having lost its purpose sucks?

With the escape analysis that Walter is implementing, surely the path of least resistance is just to turn on scope. Something akin to -transition=scopedin just to test the waters of just how much code abuses in.

I could even try this in gdc to give you an idea of what may break (or not) in an aggressive optimizer.

Telling people that their in parameter escapes and should be either const or fixed so it doesn't escape is a positive breaking change, as it improves quality of code.

I can only see this as a negative change.

@jmdavis
Copy link
Member

jmdavis commented Mar 28, 2018

Honestly, I hate in. I think that it's a bad idea to have one attribute mean the same thing as another attribute - let alone two attributes - and based on the various discussions and questions I've seen over the years, I think that it's quite safe to say that in is frequently used incorrectly and that there are quite a few folks who use it because they like the idea that it's the opposite of out and not because they really understand it and want anything like scope - even more so when you consider all of the extra places scope will have to be added as part of DIP 1000 to make code compile, which I'm sure that no one anticipated when using in.

As such, I'd love for in to be gone. However, even if we all agreed on that point (and clearly we don't, since a number of folks are quite unhappy that in now officially means just const and not const scope), I think that deprecating in now would be a terrible idea. in was changed to be just const because of all of the breakage that is bound to occur when the scope portion of it actually starts meaning something. If we deprecate it now, then everyone is going to look at their uses of in and either switch to const or to const scope. A number of those folks actually intended const scope when they used in, so they will switch to const scope, and that means that some of the breakage that was averted by changing in to const would no longer be averted.

IMHO, if we really want to get rid of in, then we should do it as part of -dip1000. It's with -dip1000 that the breakage becomes a problem, and it's with -dip1000 that code will then need to be fixed so that it really works with scope (whether that involves in or not). Now, if we're doing that, I'm sure that a number of folks will argue that we should instead just make in really mean const scope and deal with the fallout, and personally, I don't like that idea, but either way, I think that trying to get folks to stop using in before -dip1000 is ready for general use is just going to cause more problems.

Phobos is not -dip1000-compliant and some of the attempts to make it work better with -dip1000 (e.g. Jack Stouffer's attempt to use scope with std.getopt) show that some of the places that we might expect to be able to use scope won't actually be able to use scope. So, until folks can actually build their code with -dip1000 and see what happens, it's not even reasonably possible to figure out whether scope or const scope would work with a particular piece of code.

@wilzbach wilzbach added Review:Needs Work Review:Orphaned The author of the PR is no longer available and this PR can be adopted by anyone. labels Jun 27, 2018
@MoonlightSentinel
Copy link
Contributor

-preview=in was introduced recently. Should we close this?

@Geod24 Geod24 closed this Jul 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Review:Needs Approval Review:Needs Work Review:Orphaned The author of the PR is no longer available and this PR can be adopted by anyone. Severity:Enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants