Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

rename TypeInfo's init method to initializer #1403

Merged
merged 1 commit into from Dec 29, 2015

Conversation

aG0aep6G
Copy link
Contributor

@aG0aep6G aG0aep6G commented Oct 7, 2015

"init" clashes with the type property of the same name.

Adding a deprecated alias called "init" to give people a chance to
update their code. Planning to remove the alias in 2.071 to fix issue
12233.

https://issues.dlang.org/show_bug.cgi?id=15037
https://issues.dlang.org/show_bug.cgi?id=12233

@aG0aep6G
Copy link
Contributor Author

aG0aep6G commented Oct 7, 2015

I'm not sure, has 2.069 already been branched? I'm assuming it has, i.e. this PR would go into 2.070 (or later).

@jmdavis
Copy link
Member

jmdavis commented Oct 7, 2015

Well, we have quite the problem here. I don't know how often TypeInfo.init gets used in user code, but normally, symbols are deprecated for about two years before they're removed - one year as deprecated but documented and another as deprecated and undocumented. One release is nowhere near enough time for folks to update their code. Remember that many folks do not update with every compiler release, and in many cases, code bases go for a year or more without being touched.

In addition, when adding a new symbol that replaces an older one, it's generally better to not deprecate the old one for at least one release so that those who do keep up with the releases are able to build their code with both master and the latest release without getting any deprecation messages.

However, in this particular case, we're talking about renaming a virtual function. Anyone who's deriving anything from TypeInfo is going to end up with broken code because of this. An alias isn't going to fix that. The simple fact of renaming the function breaks subclasses. If it weren't for the fact that we really should make it so that no types define init, I'd argue that we're just stuck with the existing function name. So, all that the alias is going to buy us is preventing code breakage of folks calling the init function.

I don't see any sane way to do this which doesn't break code very quickly. It could be done in several steps (e.g. starting by adding initializer and implement it as calling init), but while those steps would avoid immediately breaking code, they would actually end up being far more painful in terms of the number of changes that would have to be made than to simply force folks to change init to initializer in their code. So, maybe in this case, it make sense to just alias init and leave it in for a relatively short period of time. At least the fix should be quick and easy, since all you have to do is rename init to initializer. And TypeInfo isn't used by your average D code, so this probably won't affect a lot of users. But it's definitely not ideal.

@schveiguy
Copy link
Member

Anyone who's deriving anything from TypeInfo is going to end up with broken code because of this.

And in my opinion, this is what they deserve from trying to derive from TypeInfo :)

In all seriousness, this is not a problem I think we should worry about. TypeInfo is meant to be 100% defined by the runtime, and nobody else.

Usage of TypeInfo.init however does necessitate a deprecation period. In addition, because TypeInfo.init will have a valid meaning after the function is removed, we may need to @disable init for a while (will that even work?).

I don't understand the 2 year period, that seems like a long time. I was thinking 2 or 3 releases.

@jmdavis
Copy link
Member

jmdavis commented Oct 8, 2015

I don't understand the 2 year period, that seems like a long time. I was thinking 2 or 3 releases.

In this case, that may be okay, because TypeInfo is not something used in most code. But in general, we can't have deprecation periods that short, because it breaks code too quickly. Anyone who didn't update their code nearly every release would have broken code without any warning. With something marked as deprecated for only two or three releases, someone could end up with their code breaking with no warning by simply not getting around to updating their code for about six months. And if someone's code is working, they may not need to rebuild it, in which case, they wouldn't even attempt to update it except specifically to make sure that they didn't have to make changes due to deprecations or language changes.

As it is, Walter freaked out when he updated one of his projects a while back and functions that his program used were gone - and they had been deprecated for at least two years before being removed. I suspect that he and Andrei would prefer that we never remove deprecated symbols - which wouldn't work in this case anyway, but in general, I don't like that plan, because it leaves cruft in the library along with more symbol conflicts.

We've had a two year deprecation cycle for a while now in order to minimize the risk of anyone having their code break without ever seeing any deprecation warnings. It used to be shorter, but there was complaining about that (from Andrei in particular IIRC). And for the most part, two years has been working quite well (if anything, it's sometimes longer when I miss something that should be moved along through the deprecation cycle). You don't have a lot of excuse if you haven't updated your code in two years.

So, if we are only doing two or three releases of deprecation, then that's an exception to how we normally function. We've done that from time to time, but usually not. I don't know how long would be appropriate in this case, but it would probably be okay to make it shorter given how rare it likely is for code to be messing with TypeInfo.

@aG0aep6G
Copy link
Contributor Author

Ping? Where do we stand on this? If this needs to follow a slower schedule, please advise.

@schveiguy
Copy link
Member

IMO, we should do this, but you need to at least include an alias from init to initializer, documented as to be deprecated. Then plan a schedule of when to mark it as deprecated, and then finally removal. Might as well get the process rolling now.

@dlang-bot
Copy link
Contributor

Fix Bugzilla Description
12233 Attempting to use TypeInfo.init results in a compiler error due to lack of 'this'.

@aG0aep6G aG0aep6G force-pushed the TypeInfo.initializer branch 3 times, most recently from d56ea6f to 6448525 Compare November 19, 2015 22:15
@aG0aep6G
Copy link
Contributor Author

you need to at least include an alias from init to initializer,

Had that already.

documented as to be deprecated. Then plan a schedule of when to mark it as deprecated, and then finally removal. Might as well get the process rolling now.

Ok, updated the PR. Here's the new, slower plan (also in the changelog and in the source):

  • 2.070 (we are here): rename init to initializer, add an alias init that's documented as scheduled for deprecation.
  • 2.071: Deprecate the alias.
  • 2.072: Replace the alias with an @disabled method.
  • 2.073: Remove the init method, fixing issue 12233.

@schveiguy
Copy link
Member

I'm good with the plan, but I think @jmdavis says 2 years. Given the not-often-followed-but-we're-getting-better release process, this puts it at about 8 months (a release every 2 months) before complete removal. He did say it's "probably ok" for this. Possibly the only improvement would be 2 releases with both active (i.e. deprecate in 2.072), so that there is a larger window in which one can change their code and have it compile for those on older compilers. This is especially important for those on the lagging compilers (ldc/gdc).

Anyway, nice job updating the changelog.

@aG0aep6G
Copy link
Contributor Author

Possibly the only improvement would be 2 releases with both active (i.e. deprecate in 2.072)

Updated the plan in that way. I.e.:

  • 2.071: Do nothing. Keep both init and initializer functional for a while.
  • 2.072: Deprecate the alias.
  • 2.073: Replace the alias with an @disabled method.
  • 2.074: Remove the init method, fixing issue 12233.

For review: in the changelog, and in the source.

@MartinNowak
Copy link
Member

Sounds like an OKish plan to me.
Why would anyone need init of a TypeInfo btw? Didn't we recently broke new TypeInfo and said it was OK b/c nobody is supposed to create an instance?

@aG0aep6G
Copy link
Contributor Author

Why would anyone need init of a TypeInfo btw?

It comes up in generic code, e.g. as typeof(T.init). In issue 12233, Alex Parrill gives this example: CommonType!(TypeInfo, TypeInfo) returns const(void)[].

@MartinNowak
Copy link
Member

I see, let's do it then. Lying around to long already and it should only affect people that know how to deal with it anyhow.

@schveiguy
Copy link
Member

Looks good. It's difficult to figure this out manually, but (and perhaps you've already done this) can you enable the @disable line temporarily, and show that there are no lingering usages of TypeInfo.init in druntime/phobos?

@aG0aep6G
Copy link
Contributor Author

can you enable the @disable line temporarily, and show that there are no lingering usages of TypeInfo.init in druntime/phobos?

I think that's how I found the instances. Hadn't done phobos, though. Done now: dlang/phobos#3846. Also ran the druntime tests with @disable (again) to be sure.

@schveiguy
Copy link
Member

OK, thanks, @MartinNowak did you review the code? If so, I'll pull, or you can.

@schveiguy
Copy link
Member

Looks like a merge issue with the changelog.

"init" clashes with the type property of the same name.

Adding an alias called "init" to give people a chance to update their
code.

Further deprecation plan:
* 2.071: Do nothing. Keep both init and initializer functional for a while.
* 2.072: Deprecate the alias.
* 2.073: Replace the alias with an `@disable`d method.
* 2.074: Remove the init method, fixing issue 12233.
@aG0aep6G
Copy link
Contributor Author

aG0aep6G commented Dec 1, 2015

rebased

@aG0aep6G
Copy link
Contributor Author

Ping? I'd like to get this into 2.070.

@schveiguy
Copy link
Member

I'll pull.

@schveiguy
Copy link
Member

Auto-merge toggled on

schveiguy added a commit that referenced this pull request Dec 29, 2015
rename TypeInfo's init method to initializer
@schveiguy schveiguy merged commit 2ff148b into dlang:master Dec 29, 2015
@aG0aep6G aG0aep6G deleted the TypeInfo.initializer branch January 17, 2016 22:30
nemanja-boric-sociomantic added a commit to nemanja-boric-sociomantic/swarm-1 that referenced this pull request Jan 19, 2018
TypeInfo_Class.init is deprecated and removed from the runtime (since
.init clashes with the type property of the same name). This uses
`initializer()` method which is the new way of accessing the same thing
in D2.

dlang/druntime#1403
dlang/druntime#1766
dlang/druntime#1807

https://issues.dlang.org/show_bug.cgi?id=15037
https://issues.dlang.org/show_bug.cgi?id=12233
gavin-norman-sociomantic pushed a commit to sociomantic-tsunami/swarm that referenced this pull request Jan 19, 2018
TypeInfo_Class.init is deprecated and removed from the runtime (since
.init clashes with the type property of the same name). This uses
`initializer()` method which is the new way of accessing the same thing
in D2.

dlang/druntime#1403
dlang/druntime#1766
dlang/druntime#1807

https://issues.dlang.org/show_bug.cgi?id=15037
https://issues.dlang.org/show_bug.cgi?id=12233
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants