Skip to content

Added new type alias syntax as a preference#682

Merged
MartinNowak merged 1 commit intodlang:masterfrom
JinShil:patch-1
Nov 14, 2014
Merged

Added new type alias syntax as a preference#682
MartinNowak merged 1 commit intodlang:masterfrom
JinShil:patch-1

Conversation

@JinShil
Copy link
Contributor

@JinShil JinShil commented Oct 27, 2014

This pull request is being created in an effort to aid the discussion of pull request dlang/druntime#1002.

I think it would be helpful if the D community took a position on this syntax and adopted a recommendation.

@JinShil
Copy link
Contributor Author

JinShil commented Oct 27, 2014

Also please note that much of dlang.org was updated to the new syntax about a year ago.

Discussion: #492
Final Commit: #493

@joakim-noah
Copy link
Contributor

LGTM, seems obvious since most of the docs use the new alias syntax already.

@schveiguy
Copy link
Member

I think this deserves a broader audience than a PR. I have been absent from the NG for a good portion of this year, has it been discussed there?

@jmdavis
Copy link
Member

jmdavis commented Oct 27, 2014

I think this deserves a broader audience than a PR. I have been absent from the NG for a good portion of this year, has it been discussed there?

No. And we specifically do not have much in the style guide which has to do with formatting, so I don't think that this should go in regardless of what we want to do with the aliasing syntax. The only formatting guidelines have to do with whitespace for the guide in general, and brace placement and line length for Phobos specifically. IMHO, if it doesn't affect the public API, it shouldn't go in the style guide without a really good reason. And I don't see one here.

I also see no reason to push the newer syntax like this. Both the older and newer syntax are perfectly valid and legal, and AFAIK, it's the plan that that will continue. If we want to start not allowing the older syntax in PRs for druntime and Phobos, that's one thing, but this style guide is for D at large, and I don't think that there's a good reason to try and push the whole community to use the newer syntax unless we're actually going to get rid of the older one, in which case, the older one would be deprecated in the compiler, and there would be no need to put anything in the style guide.

Certainly, if you want to try and push something like this in the style guide, I think that it needs a larger discussion, but it's my opinion that something like this has no business being in the style guide. It's not our role to try and force everyone to format their code in a particular way. How code is formatted in any particular project is up to the programmers for that project and should not be dictated by others.

@joakim-noah
Copy link
Contributor

@jmdavis, you keep talking about the two extremes of being "valid and legal" or deprecation and "force," as though those were the only two possibilities. There is however a middle way, where both types of alias syntax will be parsed by the compiler for now, but it is recommended for clarity that people use the new alias syntax. Nobody is forcing anything on anyone, it's merely a recommendation for their code. That's all this doc pull and the accompanying druntime pull enable.

From your other comment, I gather that you disagree with that recommendation also, but that is all that is under proposal.

@JinShil
Copy link
Contributor Author

JinShil commented Oct 28, 2014

we specifically do not have much in the style guide which has to do with formatting, so I don't think that this should go in regardless of what we want to do with the aliasing syntax

This is not formatting. This is akin to the recommendation for function and variable names. theFunctionName, TheFunctionName, and the_function_name are all functionally equivalent syntaxes for function names, yet the D Style still voices a preference.

IMHO, if it doesn't affect the public API, it shouldn't go in the style guide without a really good reason

Type aliases most certainly are in the public API. They are publicly exposed in many header files, specifically object.di, and it is quite often necessary to consult the header file when learning an API.

@jmdavis
Copy link
Member

jmdavis commented Oct 28, 2014

The formatting of an alias in the code has zero impact on what it looks like in the generated ddoc. What the D style guide tries to do primarily is to indicate naming conventions so that users can generally look at a D API and know what identifiers are types, are variables or functions, etc and have that be reasonably consistent across D libraries. That has nothing to do with formatting and everything to do with the public API. Which alias syntax is used has everything to do with formatting and nothing to do with APIs.

Putting anything in the style guide about preferring the newer alias syntax is essentially telling everyone not to use the older one, which makes no sense IMHO unless we're actually going to get rid of the older one, which we're not. As long as both exist and are considered valid, everyone is free to use whichever they prefer, and I see no reason to tell people to use one over the other any more than we're telling them how their functions should should be laid out or whether they should use for(;;) or while(1). That's a personal choice that has no impact on the API whatsoever and only affects anyone who's working on their code with them.

@JinShil
Copy link
Contributor Author

JinShil commented Oct 28, 2014

I think this deserves a broader audience than a PR

Posted to the forum: http://forum.dlang.org/post/qlgxmvsslbsqfcgqtedp@forum.dlang.org

@JinShil
Copy link
Contributor Author

JinShil commented Nov 3, 2014

Closing due to lack of interest.

@JinShil JinShil closed this Nov 3, 2014
@dnadlinger dnadlinger reopened this Nov 3, 2014
@dnadlinger
Copy link
Contributor

Oh, I do have interest in getting this merged.

@jmdavis
Copy link
Member

jmdavis commented Nov 3, 2014

Oh, I do have interest in getting this merged.

If this does get merged (which I'm opposed to), it needs to be in the Phobos-specific section. Putting it in the main section is essentially arguing that that the older syntax should be removed from the language, and if we're doing that, we should just deprecate it and not bother putting it in the style guide.

@dnadlinger
Copy link
Contributor

I disagree, for the reasons mentioned in the forum discussion. At this point, you are just repeating that one claim over and over without backing it up.

@jmdavis
Copy link
Member

jmdavis commented Nov 3, 2014

I disagree, for the reasons mentioned in the forum discussion. At this point, you are just repeating that one claim over and over without backing it up.

The style guide is intended for all D programmers. They're obviously not forced to follow it, but it's what we're recommending that everyone do. It's intended primarily to help keep the naming schemes of APIs consistent but does contain some minor formatting rules (like using 4 spaces for indentation). If we say in the main portion of the style guide (as opposed to the portion meant specifically for Phobos) that programmers should use the newer alias syntax, we're basically telling all D programmers not to use the old syntax, and if that's the case, we might as well deprecate it. There's no point in keeping it around long term if we're just going to tell everyone not to use it any more than we've kept around stuff like C-style function pointer declarations. Pretty much every time that we've changed syntaxes around like that, we've deprecated the old way, even if we've been slow about it, and having multiple syntaxes doesn't exactly help things, especially if we're telling everyone to not use the older one.

Now, if we put it in the section on Phobos (which currently adds only couple formatting items that we've specifically wanted in druntime and Phobos), then we'd be saying that we want to use only the newer syntax in Phobos code (for consistency or whatever argument you want to give as to why we should use only the newer syntax in Phobos), but we wouldn't be saying anything about D programs as a whole, just the official ones. So, we'd be saying what we want to see in pull requests without trying to tell the community at large what to do with their own code.

By the way, are then any plans to actually make it so that the newer syntax can even be used consistently? As I understand it, there are various places where the older syntax works, but the newer syntax doesn't (e.g. alias this doesn't work with the newer syntax). Last I heard about it Kenji wanted to keep it inconsistent for some reason.

@quickfur
Copy link
Member

quickfur commented Nov 4, 2014

alias this was left that way because it's not an actual aliasing to the symbol this, but an idiom that has a special meaning in the language. It also produces a grammatical ambiguity in the new syntax due to the abuse of the this keyword.

I disagree that we shouldn't push the new syntax; the old syntax is hard to read because it's not immediately obvious which symbol is being aliased and which symbol is being aliased to (unless you already knew it beforehand). It is also prone to infelicities like:

alias const(int) newSymbol(int x, float y, string z);

which is the kind of horrid baggage from C/C++ that we should have discarded a long time ago.

If there is any place where the new syntax still doesn't work, that's a bug that needs to be fixed, rather than passively prolonging the current unclear situation of having two syntaxes for the same thing. Deprecating it may be a little too extreme in terms of code breakage, since it's just a syntax change, but that doesn't mean we shouldn't work towards that by encouraging people to stop using the old syntax, so that we can actually move towards the point where deprecating it isn't going to trigger a backlash from current users.

@quickfur
Copy link
Member

quickfur commented Nov 4, 2014

P.S. See also: #200

@jmdavis
Copy link
Member

jmdavis commented Nov 4, 2014

Deprecating the old syntax won't break any code, just print messages (which could be really annoying if someone has a lot of aliases, but it's also easily fixable). So, if we want to deprecate it, we can either just deprecate it, or we can put a note in the changelog to warn folks a release or two before we actually do it. But I really don't think that it makes sense to tell everyone to stop using the old syntax if we're keeping it, because if we're keeping it, it's perfectly valid to use it, and there's no point in keeping something around if we're telling everyone not to use it. For Phobos, we can be more restrictive if we so choose, but if everyone is so set on not using the older syntax, I really think that we should actually deprecate it or warn people that we're going to deprecate it soon and not simply tell folks that we think that they shouldn't use it.

@quickfur
Copy link
Member

quickfur commented Nov 4, 2014

In that case, I propose we deprecate the old alias syntax.

@JinShil
Copy link
Contributor Author

JinShil commented Nov 5, 2014

So, if we want to deprecate it, we can either just deprecate it, or we can put a note in the changelog to warn folks a release or two before we actually do it. But I really don't think that it makes sense to tell everyone to stop using the old syntax if we're keeping it, because if we're keeping it, it's perfectly valid to use it, and there's no point in keeping something around if we're telling everyone not to use it.

I disagree. By deprecating the old syntax or warning users in the change log that we plan on deprecating the old syntax, we are saying that there will be consequences if they continue to use the old syntax. This pull request makes no such threats and is a much gentler way to move forward.

Eventually deprecation of the old syntax may be what we want, but that discussion can happen at a later time after the recommendation in this pull request has some time to permeate the community and users begin following the current trend.

Let's review the current trend:

  • May 2009 - User submits enhancement request likely because he believes the new syntax is an improvement to the language [1]. There is no opposition.
  • October 2012 - @9rnsr submits pull request likely because he believes the new syntax is an improvement to the language [2]. @donc voices his preference to the new syntax. @WalterBright and @andralex both approve of the change, and @andralex merges the enhancement likely because they believe the new syntax is an improvement to the language [2]. There are questions, but no opposition.
  • February 2014 - @JinShil submits pull request to update dlang.org examples to use the new syntax because he believes the new syntax is an improvement to the language [3]. @AndrejMitrovic reviews the dlang.org pull request and essentially accepts it likely because he believes it is an improvement to the documentation [3] @9rnsr merges the pull request likely because he believes it is an improvement to the documentation [4]. There is no opposition.
  • July 2014 - @andralex recommends that @Hackerpilot implement dfix with an automatic update from the old syntax to the new syntax likely because he believes such a utility would benefit D's future, and states a preference for the new syntax [5].
  • September ~ October 2014 - @Hackerpilot implements dfix and the ability to automate updating the old syntax to the new syntax likely because he believes it is important for the future development of D [6].
  • October 2014 - @joakim-noah uses dfix to update druntime to use the new syntax and submits a contribution, but encounters controversy [7]. That controversy prompts this pull request.
  • The Programming in D online book and D Cookbook both seem to have embraced the new syntax. D Cookbook has two exceptions, one of which is clarified, and the other is a code snippet from phobos.

[1] - https://issues.dlang.org/show_bug.cgi?id=3011
[2] - dlang/dmd#1187
[3] - #492
[4] - #493
[5] - http://forum.dlang.org/post/lq9sf3$3au$1@digitalmars.com
[6] - https://github.com/Hackerpilot/dfix
[7] - dlang/druntime#1002

This pull request acknowledges that the current trend embraces the new syntax, clarifies for users and contributors what that trend is, and states a preference in an effort to end future controversy.

  • New users will not have to go on tangents from their study trying to understand why there are two different syntaxes, what the differences are, and which one is recommended. They'll see the recommendation and move on.
  • Existing users can make a more educated decision about which syntax to use for new code, and, because they are not being threatened with deprecation, they don't have to worry about updating their existing code.
  • Contributors know which syntax to use when submitting pull requests, and reviewers know which syntax to enforce...without debate.
  • Authors of documentation and books will be able to make a more educated decision about which syntax to use in their material so their material will be relevant in the future and more familiar to students of the language.

I ask that this pull request be merged so we can all understand which way is forward and we can all begin moving there with less friction.

@jmdavis
Copy link
Member

jmdavis commented Nov 5, 2014

There is no controversy and no debate about which syntax to use if we deprecate the old syntax. Rather, it will clearly be on its way out, no one will use it in new code (because the compiler will yell at them if they do), and old code will be changed so that the deprecation messages stop getting printed during compilation. If we're trying to get everyone to use the new syntax, all that leaving the old syntax in does is make it so that the compiler isn't telling people, and because it's left in, there will then always be questions about the differences between them. We get by far the most controversy and friction if we leave in the old syntax, whereas if we deprecate it, the situation is very clear, and the whole problem goes away, because existing code will be changed and new code won't use the old syntax - and eventually, the old syntax won't even be legal. So, the whole problem goes away.

@quickfur
Copy link
Member

quickfur commented Nov 5, 2014

OK, so let's deprecate the old syntax now. What's the holdup?

@JinShil
Copy link
Contributor Author

JinShil commented Nov 5, 2014

Ok @jmdavis, submit the pull request to deprecate the old syntax. Good Luck!

@jmdavis
Copy link
Member

jmdavis commented Nov 5, 2014

Ok @jmdavis, submit the pull request to deprecate the old syntax. Good Luck!

I would, but that requires making changes in the compiler, and I have no experience with that. The only thing that I'm at all familiar with in the compiler is the lexer, and I don't know how deprecation works. I mostly work on druntime and Phobos. However, there is an open enhancement request to deprecate the old syntax: https://issues.dlang.org/show_bug.cgi?id=12615

@JinShil
Copy link
Contributor Author

JinShil commented Nov 5, 2014

I would, but that requires making changes in the compiler, and I have no experience with that. The only thing that I'm at all familiar with in the compiler is the lexer, and I don't know how deprecation works. I mostly work on druntime and Phobos. However, there is an open enhancement request to deprecate the old syntax: https://issues.dlang.org/show_bug.cgi?id=12615

You recommended adding a warning to the change log. Why not start there? You can also add an item to the list of deprecated features at http://dlang.org/deprecate.html. Adding a link to the deprecated features in the spec would also be helpful.

I suspect that it will meet much resistance, but if you can get it merged in the near future, I'll gladly close this pull request. If it meets too much resistance, this pull request should be merged instead.

Copy link
Member

Choose a reason for hiding this comment

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

because...
Now list the arguments.

  • It's like the assignment syntax so people without a C background don't have to learn the inverted typedef style.
  • It's easier to see what was declared.
alias important = someTemplateDetail!(withParameters, andValues);
alias Callback = ReturnType function(Arg1, Arg2) pure nothrow;

vs.

alias someTemplateDetail!(withParameters, andValues) important;
alias ReturnType function(Arg1, Arg2) pure nothrow Callback;

@schveiguy
Copy link
Member

I don't think the old syntax needs to be deprecated to state we prefer the new syntax over the old. I also think deprecating the old syntax does not help at all -- it breaks code for no good reason. It would be like deprecating non-camel-case symbols or something. The old syntax DOES provide a way to alias multiple symbols at a time, the new syntax does not. The argument that we must either embrace deprecation or not pick a side is a false choice. We can dictate a preference, but not enforce it via the compiler. It's a style preference, nothing more. Like tabs vs. spaces.

@MartinNowak has good arguments for why we should adopt the new syntax for our code.

All that being said, I'm so unconcerned with this that I'm not going to say I'm for or against this change. It's so minor, and doesn't really do much -- the functionality of alias is so much more relevant than its syntax, that I think anyone learning alias will learn whatever syntax as a side effect of learning its power. Both sides seem to me to have brought their mountain climbing gear to conquer some molehills.

Added elaboration based on comments received

Fix syntax errors
@MartinNowak
Copy link
Member

It's says preferred for better readability, which is pretty weak, but makes it clear which one should be used for new code.
Also we didn't add a new syntax without a reason.

MartinNowak added a commit that referenced this pull request Nov 14, 2014
Added new type alias syntax as a preference
@MartinNowak MartinNowak merged commit 26f94d5 into dlang:master Nov 14, 2014
@MartinNowak
Copy link
Member

Don't sweat the small stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants