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
Issue 5481 - Support deprecated("message") #345
Conversation
This patch adds two new forms of deprecation: `deprecated("message")` and `deprecated("message", soft)` The first behaves the same as the old deprecated, but also prints a message after the standard error. The second instead issues a warning.
That's essentially what I'm looking for but not quite. The main question is whether soft deprecation should be a warning or just always printed when the symbol is used. We've been treating it as "always printed," but some folks have found that to be annoying. For anyone who ignores warnings, soft deprecation as done here doesn't add anything, and they still get hit with the hard deprecation out of the blue if they haven't read the documentation or changelog. And for anyone who always uses -w, they effectively get hit with hard deprecation immediately. It only works as "soft" when you compile with -wi. So, I'd be inclined to have "soft" deprecation always print the message when the symbol is used rather than making it a warning. Otherwise, the usefulness of the soft deprecation is limited. Certainly, having a message with If you want more feedback on the changes though, you're going to have to post about it in the newsgroup. Only the dmd devs are likely to notice a pull request to dmd. I only noticed it because I was CCed on the enhancement request. Andrei probably won't notice it at all. |
I don't think it's acceptable to have soft deprecation message always print, with no way to disable them. When this was last brought up on the newsgroup, many people said the same thing. Given that, there are a few ways to do it.
I think 1 is the best, 2 requires a new compiler switch and is probably overkill, and 3 introduces a new kind of 'non-fatal' warning which is rather against the spirit of D's warnings. Is there another way you can think of that can keep sides happy? Thanks for the feedback. I'm asking here first, and you particularly, because phobos is the major user of deprecated and you seem to be in charge of managing deprecation in phobos. I do want to try and reach an agreement before taking this to the newsgroup again. I was under the impression @ tagging github users would send them a message. I'll post a link on the phobos list. |
I fully understand people not wanting to be flooded with messages, and if too many get printed, they get ignored anyway. However, the problem with making it a warning is that the only case that it really does anything is when you compile with The ideal situation probably be something like #2, since then you actually see the messages, but then if people get annoyed with them, they'll just use the new flag to disable them, and they won't do any good anyway. So, it's not entirely ideal. However, it does make soft deprecation act more or less like hard deprecation except that you only get a message instead of an error. So, that's good. #3 goes against how warnings normally work with dmd, so we probably don't want to do that. I think that the reality of the matter is that you're never going to make everyone happy, so the question is how best to get the message across that programmers are going to need to change their code and yet not annoy them in the process. #2 definitely seems like the best route for that to me, but it is further complicating things by adding yet another compiler flag. So, while #2 might be the best, we may want to go with #1 just to simplify things - especially since in the long run, deprecation in Phobos should be rare. |
See also: #248 - I think this has some relevance to this discussion. |
I'm surprised no one suggested a version identifier for this (e.g. |
How about to have 3 versions, |
@CyberShadow: Isn't that basically the same as adding a new compiler switch, but by creating a new abuse of version syntax? ie. Causes the same complication to the compiler's interface as a new switch, except worse? @kennytm: That would still require a way to silence deprecation messages, and I'm not sure adding an extra stage of deprecation is necessary. @jmdavis: |
I meant in the context of implementing soft deprecation as a template. Currently, soft deprecation in templates unconditionally issues a pragma message. A |
Ah, ok. That might be worth doing if this pull request (or another solution) isn't accepted by the next release. |
Okay. Assuming that when compiling with So, IIUC, what this patch does is
Assuming that that's what it does, and I haven't misunderstood, then I can accept that solution. It's not quite what I'd like, but it works well enough considering that deprecation should be fairly rare. |
I may have misunderstood something here, but it seems this whole approach is one step too soft. We currently have a big problem: when something has been deprecated and has been compiled with -d, then when it is removed, it just fails to compile -- and it doesn't give you a hint about what to do. This has been happening with things in the test suite, for example. We need a deprecation which is harder than normal: it must generate an error even when compiled with -d. (A 'your time is up' error). Note that once you start compiling with -d, if anything else you're using gets deprecated, you won't be informed at all. So, we've got these "scheduled for deprecation" messages, so that people don't have to do the dangerous activity of using -d. |
Well, the point of soft deprecation as generally discussed is to inform users that a symbol is going to be deprecated and that they're going to have to have to change their code while not breaking their code (which is why making it a warning instead of just a message isn't altogether ideal - anyone compiling with Hard deprecation as has been generally discussed is normal deprecation. And as that currently works, if you use I do not think that the current situation is ideal, nor do I think that this patch is entirely ideal, but I really don't know how to improve it beyond this. Ultimately, you need something like We could just say "screw it," hard deprecate things immediately (that is, deprecate them with the current type of deprecation), and then have a harder level of deprecation where the symbol is unusable - even with So, I don't know. I think that this pull request is an improvement, but I don't see a really great way to fix this problem in general. You either end up pestering the user with messages to fix their code and/or hitting them with breaking changes that they weren't expecting and force them to change their code immediately. We need a fairly simple and reasonable approach to this which at least mostly works, but hopefully in the long run deprecation is fairly rare so that it's not a big issue regardless. |
Walter doesn't want people to be forced to use -d with no warning. (source) Adding harder deprecation seems to me like it's solving a different problem: giving useless error messages once something has been completely removed. It sounds useful, and this patch is about 6 lines away from giving you that, but it's not the same thing. |
If the "change" simply involves compiling with -d, I don't think that's at all unreasonable.That can be done in less than a minute, and doesn't require any thought. But at the moment, -d is unusable, it leaves your code vulnerable. Which means that at the moment, as soon as something is marked as deprecated, you are practically forced to change it. normal/soft: I think documentation/normal/hard is better than soft/normal/sudden death. |
Well, I don't think that there's much question that adding a message to |
Then I really don't understand. I saw the role of -d to be "I acknowledge that this functionality is deprecated, but I need more time". |
Well, I believe that Walter's stance on Personally, I hate Maybe the better solution would be to make it so that we have soft deprecation as in this pull request (and |
Actually, with that solution, you still wouldn't get a message with |
What I think should happen. If you use -d, don't show deprecation messages. If you don't use -d soft deprecation doesn't stop compilation, but hard does. Use -d with -w or -wi to see deprecation messages but don't stop compilation. |
I get the feeling that this is being way over-engineered. I'd prefer just a simple optional message for the deprecated keyword, and leave the rest as is. We have warnings, and deprecations, and building more layers of complexity on top of this just does not seem worthwhile. The deprecation message should simply say what the user should do to correct the problem. |
I definitely agree that this risks being overengineered. However, I also think that we need to make changes beyond just adding the ability to give a message to And because code is effectively immediately broken when something is deprecated, you wanted us to schedule stuff for deprecation first - which we've been doing. But the only way to give any message beyond putting it in the documentation and changelog (where it's probably not going to be seen by very many people) is to use pragmas, which only works in templated functions, and has proven to be overly annoying for many people. So, if you stop using pragmas as we have been and we only put information on stuff being scheduled for deprecation in the changelog and documentation, then most people won't notice it, and they won't change their code. So, when we actually deprecate something, it's not much better than having immediately deprecated it. And if people don't want to change their code immediately, they then use In the long run, deprecation in Phobos should be fairly rare, so the deprecation mechanism won't be as a big a deal, and so having |
One man's over-engineered feature is another man's differentiating feature that makes D attractive. We need to reckon that one way or another we need to be on top of replacing old mistakes with new mistakes :o). I have a different idea that automates the deprecation process. Consider:
When the compiler encounters that, it compares the seconds string with a string built like yyyy/mm/dd from the current date. (It's a straight string comparison, nothing fancy.) If the string in the deprecation message is greater, allow use of the feature with -d, otherwise unconditionally disallow. The error message can use all this info to be very nice, e.g.:
and later on:
|
What's "current date"? Surely not the date from the system clock during compilation? Dates in deprecation messages are simply a way to refer to future DMD releases without having to use future version numbers. Some people may want to avoid upgrading specifically to avoid the hassle of updating their code base to work with the newer versions - breaking that would be a horrible idea. |
Yes, current date is date at compilation time. I don't understand the rest of your argument. |
You are suggesting that compilation of programs with the same version of DMD/Phobos to break by itself over time, right? |
That is the intent. |
...And you really don't see anything wrong with the idea that things break by themselves without the user doing anything? |
In general, I like Andrei's idea, although dmd does not have any date parsing |
Sorry to sound arrogant, I'm not sure if I'm the only one who sees the problem with this suggestion or simply the consensus being that it's a non-problem. Perhaps I should expand on my thoughts a bit. I think that making any behavior depend on the system clock is a very bad idea. Currently, user code breaks when the user performs an action which implies that things might break: update DMD/Phobos. The user usually performs this action at a point where they are ready to handle the consequences: be prepared to be annoyed by new pending-deprecation messages, edit their makefile, finally stop using features that have been pending deprecation, etc. Making things stop working when the system clock rolls over some value is pretty much putting a time bomb in D. Even though we can expect that the majority of users will want to stay up-to-date with current compiler/library versions, it would be presumptuous to assume that about all users. For all you know, someone may be stuck using an old DMD version due to a very specific compiler regression that no one wants to fix, or their codebase depends on a specific quirk of that compiler or library version, or they simply don't have the resources to migrate their codebase away from the deprecated functionality. This is half the trouble. In some circumstances (automated builds / build farms), pending-deprecation messages may be hidden from users. It's very uncommon for software which can (and, to some extent, is designed to be) used in an automated manner to stop working depending on the value of the system clock. I imagine this would be an unwelcome surprise to such users. Given my above points, I don't see how you could consider this objectively a good idea, considering its advantages are relatively minor. |
Much as I like Andrei's idea in concept, I'm going to have to agree with CyberShadow here. The problem with Andrei's proposal is that it's not at all version-based. If I have version 2.055 of dmd/druntime/phobos and my code compiles with it, I would expect that that code would still compile with that code 2 years from now. For any code that's not actively maintained, that's going to be a problem. Having it not compile with 2.070 would be fine, because it's provides a new version of the libraries (and the compiler itself could contain bug fixes or feature changes which change the behavior or render uncompilable the old code), but 2.055 should still be able to compile the same code. Much as I like the idea of providing a way to make functions automatically fully deprecate themselves, I think that making it time-based as opposed to version-based is a problem. Simply making There have been times at work where we have had to compile code for an older product that no one has touched in months, if not years - code which has not been being maintained - and if that code had compiled the last time that it was worked on but doesn't now simply because the date changed, that would be a big problem. The environment (and thus the compiler version) would be the same, because we're strict about our build environments, but code for older products is generally not updated unless there are bug fixes that have to be made, and suddenly having to worry about porting code from older versions of functions or modules to newer functions (which may or may not be trivial) just to do a bug fix isn't really acceptable IMHO. |
On 9/10/2011 2:27 PM, Vladimir Panteleev wrote:
You make a sound argument. I agree. |
I agree with the "time bomb" argument though I'm not as sanguine about it as others. An alternative would be to compare the deprecation date against the compiler's build date. That means in the given example that a compiler released after 2012/02/14 will refuse to compile that code. Walter, there's no need for date parsing. The beauty of the yyyy/mm/dd format is that you can compare dates just as strings. Databases actually do that to great effect. |
Using the compiler build date would work quite well for Phobos (and would be quite cool actually), but it doesn't scale beyond that. Should when a function in one of my personal projects is fully deprecated depend on the version of the compiler or Phobos that you're using? I would think not. But with your proposed date scheme, it would. |
On 9/10/2011 4:20 PM, Andrei Alexandrescu wrote:
Sure, but then the user uses yyyy-mm-dd format, or mm-dd-yyyy format, or Databases might be able to get away with it because they are not using free |
I agree with CyberShadow,, but I disagree with this: Currently DMD still has plenty of bugs and a good reason to upgrade is to get the latest set of bugfixes, but you might not be prepared to upgrade to a newer version of Phobos yet. Unfortunately Phobos and DMD releases are tied together, so with new bugfixes you potentially get new deprecation warnings. |
I'm not sure what to do about the date/soft parameter, but I don't like the idea of -d not allowing deprecated code to compile. For one thing it makes documenting -d confusing. Without -d all deprecations hard/soft should show a message. With -d soft deprecation would not print a message but hard ones still would. |
I don't agree to the time bomb. I think that the soft deprecation is unnecessary, too. |
@yebblies We can all agree that the ability to give deprecated a custom message is a definite improvement. So, I suggest that you close this pull request and create a new one where that improvement is included and only that improvement. If we can agree on other improvements later, then we can add them then, but we might as well get the improvement of the custom messages in so that we can start taking advantage of it. |
I'm going close this in favor of a patch that only adds an optional message to deprecated. It seems like once this is done, phobos should move to a different deprecation process:
One alteration might be adding a stage between Deprecated and Removed -
I understand this isn't anybody's idea of perfect, but are there any huge problems with this idea? |
Walter seems to be against pretty much anything which complicates |
Yeah, I did this yesterday: #463 Regardless of other issues, I haven't seen anyone arguing against adding the optional message to deprecated. I like @donc's suggestion of having a stage where you must modify your code, but the old code is still available for copy-paste rescue. Rather than complicating deprecated to support this, @disable already does it. |
Issue 7055 - to!float("INF2") == 2
This patch adds two new forms of deprecation:
deprecated("message")
anddeprecated("message", soft)
The first behaves the same as the old deprecated, but also prints a message after the standard error.
The second instead issues a warning.
This patch is backwards compatible with the current deprecation system.
http://d.puremagic.com/issues/show_bug.cgi?id=5481
@jmdavis I know this isn't exactly what you wanted. It can easily be changed if there's any sort of agreement. (@andralex ?)