DIP1003: Remove body as a Keyword
#48
Conversation
|
Thank you! I will provide initial feedback by the end of this week, |
|
As I understand the DIP process, now is not the time to comment on the technical merits/demerits of the proposal. If that's not the case then let me know. What we have right now is a DIP which recommends a breaking change, but then offers up several possible solutions to mitigate it and does not recommend one over the other. This feels wrong to me. I understand the situation we are in where Walter and Andrei may not agree with your deprecation process but they agree with the de-symbolizing of I curious to hear Dicebot's opinion. |
|
I have recommended a solution. See the Breaking Changes section:
I'll make it more clear that I recommend option 5 or 6. |
It is not discouraged per se - simply not the main focus at current stage. But overall it is better to comment on it later after NG announcement, it will get more attention / dicussion that way. |
There was a problem hiding this comment.
Commented with some suggestions on both readability and content. Overall what I think it currently lacks most is focus in a way that it presents many approaches without putting too much effort into selling either of them. It risks becoming victim of the paradox of choice that way.
Considering all breaking change policies we are trying to enforce, only two essential approaches pass the checklist from my PoV:
- conditional keyword
- removing body altogether with very long (2+ years) deprecation period before it becomes available
If you agree with that, it may be better to focus on investigating just those two.
| - [Usage of the Body Keyword](http://dlang.org/spec/function.html#BodyStatement) | ||
| - [Usage of the Function Keyword](https://dlang.org/spec/expression.html#FunctionLiteral) | ||
| - [Previous Discussion from 2011](http://forum.dlang.org/thread/imdro4$286k$1@digitalmars.com) | ||
| - [Current Forum Discussion on the usage of body in user code](http://forum.dlang.org/thread/nyrosepldsxabewksehb@forum.dlang.org) |
There was a problem hiding this comment.
As document will stay visible online for a while, it is better to separate all forum discussions in one sub-group and mark them with year instead of using words like "current"
| - [Example of the word body used in an external C library ported to D](http://forum.dlang.org/post/lxdsvhygsaesjmkmavqp@forum.dlang.org) | ||
| - [Example 1 of the word body used in an external C++ library ported to D](https://github.com/d-gamedev-team/dchip/blob/55f43e5f0cf67c8bc190711b69eb16230fa6188e/src/dchip/cpBody.d#L184) | ||
| - [Example 2 of the word body used in an external C++ library ported to D](https://github.com/d-gamedev-team/dbox/blob/6f81fe065abec1e7def44fc777c5d8e9da936104/examples/demo/tests/bodytypes.d#L103) | ||
| - [Example 3 of the word body used in an external C++ library ported to D](https://github.com/rcorre/chipmunkd/commit/d6bde5b649c70a53f4295f522e660fae3c1e740f) |
There was a problem hiding this comment.
I'd suggest to move links with problematic examples to "rationale" section.
| The proposed changes to D are very simple, and are as follows: | ||
|
|
||
| 1. Remove `body` as a keyword | ||
| 2. Allow the usage of the word "body" as a symbol name. |
There was a problem hiding this comment.
Those 2 points are not enough to sufficiently described required change in semantics. You need to refer to cases proposed in "breaking changes" section which answer what happens with old contract syntax to complete it.
| 5. Add `function` as an optional keyword in the place of `body` and schedule the `body` keyword for deprecation. After this period allow `body` as a symbol name | Yes | Yes | Yes | Moderate | Moderate | | ||
| 6. Allow the function body to optionally be annotated with `body`, but do not require it. Schedule the `body` keyword for deprecation. After this period, allow body as a symbol name | Yes | Yes | Yes | Moderate | Moderate | Makes function contracts 1 line shorter | ||
| 7. Schedule `body` for deprecation and then removal. After this period, add `function` in its place and allow `body` as a symbol name | Yes | Yes | No | Difficult | Moderate | | ||
| 8. Schedule `body` for deprecation and then removal. After this period, disallow its use to denote the function body and allow `body` as a symbol name | Yes | Yes | No | Difficult | Moderate | Makes function contracts 1 line shorter |
There was a problem hiding this comment.
Please remove all options that imply skipping deprecation process and breaking user code immediately. Such approaches have exactly 0% chance of being approved and thus only distract from essentials.
There was a problem hiding this comment.
On the same topic - instead of listing all combinations of possible approaches, it would be more practical to split required changes into independent blocks and list solutions for each of blocks. Something like this:
1) Existing usage of "body" in contracts:
* deprecated, replacement suggested
* remains valid
2) New suggested way of marking function body in contracts:
* no keyword (just brackets)
* "function"
* conditional "body" keyword
3) How soon "body" can be used a symbol?
* immediately (only applicable with conditional keyword case)
* after "body" keyword deprecation term has passed
.. and after that pick few most practical combinations in your opinion with explanation why they are best.
There was a problem hiding this comment.
I've only included those options for completeness' sake. Later on I specifiy that these are not recommended.
I'll look at splitting it up as you suggested.
|
|
||
| ### Rationale | ||
|
|
||
| Many D programmers complain about `body` being a keyword in D (see links section for relevant discussion). It is a commonly used |
There was a problem hiding this comment.
AFAIR, you can use relative link to other section for ease of reading, something like this: [links](#Links)
| ### Breaking changes / deprecation process | ||
|
|
||
| Depending on how these changes are implemented, there will be little to no breakage, | ||
| and any breakage that _does_ occur will be done through the established deprecation |
There was a problem hiding this comment.
I fail to see how this conclusion comes from previous statements. The way I read it, "minimal to no breakage" is only applicable to conditional keyword approach. All other approaches will cause a huge breakage and would need to undergo deprecation stage of several years at least before taking full power.
|
|
||
| Depending on how these changes are implemented, there will be little to no breakage, | ||
| and any breakage that _does_ occur will be done through the established deprecation | ||
| process for breaking changes (first a warning, then deprecation, then removal). |
There was a problem hiding this comment.
Current process is "deprecation -> (optionally) warning -> removal". Do you remember where you have noticed the process documented as you describe? We should have cleaned all cases by now but few could be left.
| process for breaking changes (first a warning, then deprecation, then removal). | ||
|
|
||
| It is the author's opinion that option 4 is the ideal path forward for implementing this proposal and minimizing breakage. However, this | ||
| may not be feasible as it requires the implementation of conditional keywords and the effort to implement them is currently unknown. Likewise, |
There was a problem hiding this comment.
It feels like some research regarding feasibility of conditional keywords is necessary to be included into the DIP (at least in a form of links to existing NG discussions). That will certainly affect evaluation of proposal a lot.
There was a problem hiding this comment.
I didn't bother because Walter is against conditional keywords, so they will never be implemented. Actually it might be better to just remove this suggestion as it won't happen.
There was a problem hiding this comment.
I think it has a much better chance if used as a tool for better deprecation process and not come into language as full blow precedent. That is solution I'd prefer myself to be honest (use conditional keyword to start using "body" as a symbol early, but also deprecate its usage in contracts to be completely dropped out of keywords later).
Linking Walter comments on topic of conditional keywords would be useful too.
There was a problem hiding this comment.
IMO conditional keywords need a whole other DIP to address them. I'd prefer to keep this DIP focused on the issue of removing body.
There was a problem hiding this comment.
I feel like it is useful/necessary. Pretty much the only practical alternative is to deprecate current body keyword and wait 2+ years before it can be reclaimed. Surely, extra effort to try pushing for "fast and smooth" approach is worth saving everyone interested that 2 years.
If you don't want to do so, at the very least you need to make clear remark of why this path is not considered, with links to Walter stance on topic. "effort to implement them is currently unknown" is simply not a good wording for a DIP.
There was a problem hiding this comment.
Well, it's true. Only compiler hackers know roughly how much effort would have to go into implementing conditional keywords. I'll see if I can find some stuff Walter's said.
There was a problem hiding this comment.
Only compiler hackers know roughly how much effort would have to go into implementing conditional keywords.
It shouldn't be mentioned in DIP at all in that case - implementation complexity for compiler internals should not effect approving the proposal. On the other hand, if there is a semantical complexity that is not related compiler internals, it has to be explained.
|
|
||
| ### Examples | ||
|
|
||
| How "body" might be used as a variable name in DOM-based code: |
There was a problem hiding this comment.
I think one good argument in favor of this DIP could come from examples of confusing compiler messages when one naively attempts to use "body" as a variable/argument name. As contract programming is not widely used in D, I have seen newbies being terribly confused by such occurrence.
It can be added to rationale too.
| } | ||
| ``` | ||
|
|
||
| Examples of how code will change with this proposal: |
There was a problem hiding this comment.
You should link these examples from relevant entries in description section.
It is ok for DIP to propose few alternative (but related!) solutions for same problem as long as all of them are presented in good enough details and document structure allows to easily comprehend which parts are orthogonal. In that case A&W can simply pick one during approval. However I agree that it is not good for options to be so many that picking one becomes a creative act of its own. Also see my paradox of choice remark in review comments. |
|
Well, in fairness I recommend one of two options and reject the others for various reasons. I'll look at pruning some of them, though. |
|
If it helps as an example I was able to get druntime, phobos, and dmd v2.071.2 compiling and running by removing "body" as a keyword and having "function" used as its' replacement. Only needed to use a relatively simple regex, took about an hour or two to make all the changes and double check. Only thing is I didn't go over changes in documentation for DMD, though for DMD's case, it'll probably have more that needs to be changed than any other codebase's documentation. skl131313/phobos@bd7a84b |
… in paragraph form.
… example of shorter contract syntax with Option 3.
…ions. Separate the examples and have them link back as well.
|
I have made some of the requested changes. Still to do is add links to statements from Walter saying that he opposes conditional keywords, as well as adding a section on error messages when trying to use |
Ping. Any updates here? |
|
None yet. I have some time this weekend so I will get to this then. |
|
Well, I have been unable to find anything substantive other than this recent post by Walter. I have created a thread on the D mailing list asking him to make an official statement. |
|
Still todo: a section on error messages when trying to use |
|
Sounds reasonable. I'll wait if Walter provides more input on topic and make one final review pass - overall this DIP looks to be in quite decent shape. |
| to DMD on the basis that they will complicate the parser. | ||
|
|
||
| The grammar will not have to change if this option is implemented. | ||
|
|
||
| #### Option 2: Allow `function`, replace `body` after deprecation | ||
| Add `function` as an optional keyword in the place of `body` and schedule the `body` keyword for deprecation. After this | ||
| period allow `body` as a symbol name. This option will require some user effort to adjust their code and does not have | ||
| a migration path that is as easy as [Option 1](#option-1-conditional-keyword). However, it also does not require that conditional keywords be implented. | ||
| a migration path that is as easy as [Option 1](#option-1-contextual-keyword). However, it also does not require that contextual keywords be implented. |
|
Thinking more about this, I don't think it's worth it to include arguments relating to confusing error messages when trying to use the word It just all around seems like a very weak argument, so I don't think it's worth including. |
|
I don't think this DIP can justify the breaking change it suggests. It doesn't bring any significant value but does break a bunch of existing code. And you can just prefix, suffix or change case of your name to e.g. body_ like you have to with all other keyword which isn't a big deal. |
|
@MetaLang oops, I though I have already commented here but looks like it was only my imagination :X FYI: I am waiting for Andrei to publish his response on existing merged DIPs to use it as a sanity check before merging. Don't have any specific requirement on my side right now. |
|
Ok, thanks. What's the timeframe for him responding to the merged DIPs? |
|
The deadline for the merged DIPs was two weeks ago - so I hope it will be On Tue, Nov 15, 2016, 13:42 MetaLang notifications@github.com wrote:
|
|
Initial deadline was this Monday and it is obviously missed. I have just asked Andrei for an update. |
|
Andrei's judgement of last merged DIPs: https://github.com/dlang/DIPs/blob/master/DIPs/DIP1001.md#review |
This is the initial draft of DIP1003 for criticism and verification of acceptance criteria.