Skip to content

Fix issue 17408: scope and in are considered redundant#10168

Closed
Geod24 wants to merge 2 commits intodlang:masterfrom
Geod24:in-transition
Closed

Fix issue 17408: scope and in are considered redundant#10168
Geod24 wants to merge 2 commits intodlang:masterfrom
Geod24:in-transition

Conversation

@Geod24
Copy link
Member

@Geod24 Geod24 commented Jul 13, 2019

This fixes the issue at hand, although I am not really sure what we should do with in.
As it stands should it just be deprecated or should we add -transition=in to make it work again ?

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @Geod24!

Bugzilla references

Auto-close Bugzilla Severity Description
17408 normal scope and in are considered redundant

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#10168"

@WalterBright
Copy link
Member

Fixing the behavior to - what?

@Geod24
Copy link
Member Author

Geod24 commented Jul 13, 2019

Fixing the behavior to - what?

This pull request make it so that someone writing:
void foo(in scope char[]) will not get an error message. Someone writing void foo(in const char[]) will.

For the future (nothing that this PR do):
What is the point of having in in the language if it's just an alias for const ?
That's technical debt. So either we make it useful or we deprecate it.
Make it useful:

  • in == const scope is very much into people's mind (that's how it always was). We could provide a -transition=in flag to restore this behavior without breaking any code
  • Other options ?

@12345swordy
Copy link
Contributor

private StorageClass appendStorageClass(StorageClass storageClass, StorageClass stc)
{
if ((storageClass & stc) || (storageClass & AST.STC.in_ && stc & (AST.STC.const_ | AST.STC.scope_)) || (stc & AST.STC.in_ && storageClass & (AST.STC.const_ | AST.STC.scope_)))
static bool isIn (StorageClass stc)
Copy link
Contributor

@JinShil JinShil Jul 14, 2019

Choose a reason for hiding this comment

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

I think this should be a public utility function accompanying the STC declaration at

enum STC : long

@JinShil
Copy link
Contributor

JinShil commented Jul 14, 2019

Other options ?

I offered the following suggestion at https://forum.dlang.org/post/aqkrrclvdgqeiusccgyt@forum.dlang.org

Anywhere in is used we display the following deprecation warning message...

Deprecation: in is currently equivalent to const. Some time after v2.xxx, in will be changed to scope const. To keep the current semantics, replace in with const. To silence this message change your code to scope const or simply ignore this message until v2.xxx. See https://dlang.org/spec/function.html#parameters for more information.

... add a changelog entry and update the documentation to explain the change.

When v2.xxx is released, we're all set with a better D.

Choose "v2.xxx" to be whatever works to achieve consensus.

@Geod24
Copy link
Member Author

Geod24 commented Jul 15, 2019

@JinShil : I will be strongly opposed to any modification that requires double modification of the source code. Likewise, I'm strongly opposed to any modification that rely on you ignoring a message. It is just user-unfriendly.

@JinShil
Copy link
Contributor

JinShil commented Jul 15, 2019

I will be strongly opposed to any modification that requires double modification of the source code.

No double modification is required. Users can keep scope const indefinitely without any problem.

I'm strongly opposed to any modification that rely on you ignoring a message.

It doesn't rely on users ignoring the message. It simply offers it as an option. If users don't wish to ignore the message, they can modify their source code.

Also, an important realization about my suggestion is that whatever problems one might see with it are temporary. The entire issue, and its problems will be non-existent after 2.xxx is released which could be as short as 1 year. If my suggestion would have been approved at the time I made it, we wouldn't even be having this discussion right now.

@JinShil
Copy link
Contributor

JinShil commented Jul 15, 2019

If users don't wish to ignore the message, they can modify their source code.

I wouldn't be opposed to removing the option to ignore the message, if it would help get this issue resolved. The deprecation message could then read.

_Deprecation: in is currently equivalent to const. Some time after v2.xxx, in will be changed to scope const. To keep the current semantics, replace in with const. To silence this message replace in with scope const.

Effectively, what my suggestion is doing is discouraging the use of in until such time as changing its semantics will cause little to no breakage.

@Geod24
Copy link
Member Author

Geod24 commented Jul 15, 2019

So why not simply remove in if we are going to discourage everyone from using it for one year ?

@JinShil
Copy link
Contributor

JinShil commented Jul 15, 2019

So why not simply remove in if we are going to discourage everyone from using it for one year ?

Yes, that's the fundamental question I was hoping to get an answer to in my forum post: "What is the long-term plan for in?". Unfortunately, it was treated as an insignificant issue and ultimately ignored.

So, you tell me. What do you want in to be in the long term? Do you want to remove it from the language altogether, or do you want to fix it for future use? The answer to that question will inform the right direction. I'm suggesting the latter: temporarily discourage the use of in until such time as it can be properly implemented without risk of too much breakage.

@JinShil
Copy link
Contributor

JinShil commented Jul 15, 2019

Another option is to make in equivalent to scope const today. @atilaneves seemed to imply that in his forum post at https://forum.dlang.org/post/oljtpjapckrvnhcfhxyl@forum.dlang.org I'd be OK with that. Maybe he could make an executive decision to expedite a proper implementation of in. Or maybe we just deprecate in-as-const for 1 release, rather than a year. I'd be OK with that too.

@MoonlightSentinel
Copy link
Contributor

Should we close this given -preview=in ?

@12345swordy
Copy link
Contributor

I think so. @Geod24 any objections?

@Geod24
Copy link
Member Author

Geod24 commented Jul 14, 2020

The issue is still present, and could be correctly fixed, but I guess we can live with this.

@Geod24 Geod24 closed this Jul 14, 2020
@Geod24 Geod24 deleted the in-transition branch July 14, 2020 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants