Skip to content

Conversation

@rainers
Copy link
Member

@rainers rainers commented Feb 28, 2018

…rd has been deprecated

defer deprecation message into semantic stage so it can still be versioned out.

Please note that the delete declaration is not deprecated, so the message should probably better say "delete expression" instead of "delete keyword".

@dlang-bot
Copy link
Contributor

dlang-bot commented Feb 28, 2018

Thanks for your pull request, @rainers!

Bugzilla references

Auto-close Bugzilla Severity Description
18530 regression [Reg 2.079] src/rt/tracegc.d(43): Deprecation: The delete keyword has been deprecated

@rainers
Copy link
Member Author

rainers commented Feb 28, 2018

Hmmm, master uses

"The `delete` keyword has been deprecated.  Use `object.destroy()` (and `core.memory.GC.free()` if applicable) instead."

while stable has

"The `delete` keyword has been deprecated.  Use `object.destroy()` instead."

Which one is the last consensus?

@JinShil
Copy link
Contributor

JinShil commented Feb 28, 2018

Which one is the last consensus?

master. #7947

// 1. Deprecation for 1 year
// 2. Error for 1 year
// 3. Removal, "delete" can be used for other identities
if (!exp.isRAII)
Copy link
Contributor

Choose a reason for hiding this comment

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

It's important that when the deprecation period is over, the keyword is removed from the parser. Please add a comment about that here so it doesn't get overlooked.

Copy link
Member Author

Choose a reason for hiding this comment

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

added "keyword" to the comment.

@JinShil
Copy link
Contributor

JinShil commented Feb 28, 2018

It's also worth noting that once the delete keyword is removed, the code in tracegc.d will no longer compile. So, I don't think this is the way to go. We should just comment out that code in tracegc.d; it's already versioned out with version(none) anyway, which is functionally no different than a comment. Also, that code is just an illustration for testing.

@JinShil
Copy link
Contributor

JinShil commented Feb 28, 2018

Please note that the delete declaration is not deprecated, so the message should probably better say "delete expression" instead of "delete keyword".

It's my understanding that we want users to be able to use delete for their own identifiers. Therefore, I believe it is indeed the keyword, and everything that follows from it, that is being deprecated.

@rainers
Copy link
Member Author

rainers commented Feb 28, 2018

master. #7947

Good, that's what I started from, too.

We should just comment out that code in tracegc.d

I agree it's not really an issue for tracegc.d, but having to maintain conditional code for different versions and compilers with delete and __delete will be a PITA if the parser stumbles over the mere existence.

It's my understanding that we want users to be able to use delete for their own identifiers.

Shouldn't the delete declarations (class deallocators) be deprecated now, too? Probably also class allocators?

…rd has been deprecated

defer deprecation message into semantic stage so it can still be versioned out
@wilzbach
Copy link
Contributor

I agree it's not really an issue for tracegc.d, but having to maintain conditional code for different versions and compilers with delete and __delete will be a PITA if the parser stumbles over the mere existence.

That's a good point. Maybe we should put __delete in undead too?

Shouldn't the delete declarations (class deallocators) be deprecated now, too? Probably also class allocators?

AFAICT they are a relict from D1.
Also they are on the list: https://dlang.org/deprecate.html#Class%20allocators%20and%20deallocators
I assume it was never deprecated because of the missing __delete or just no care bother enough to deprecate.

@rainers
Copy link
Member Author

rainers commented Feb 28, 2018

Maybe we should put __delete in undead too?

__delete can be helpful if it works for older compiler versions, too (and for all use cases). I guess a simple wrapper template using a mixin might also work in most cases.

@MartinNowak
Copy link
Member

Errors in parsers are generally not helpful, as they fail to early and prevent backwards compatible code. So I agree with @rainers here.

@JinShil
Copy link
Contributor

JinShil commented Mar 3, 2018

Shouldn't the delete declarations (class deallocators) be deprecated now, too? Probably also class allocators?

Yes, there is a lot of unfinished business with regard to following through on deprecations. It one of my top priorities, which motivated pushing the the deprecation of delete in the first place.

I would definitely support deprecating delete declarations (new declarations too?), but they need to go in a separate PR in support of https://dlang.org/deprecate.html#Class%20allocators%20and%20deallocators and a dlang.org PR should be created to properly fill out the deprecations table.

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.

5 participants