-
-
Notifications
You must be signed in to change notification settings - Fork 742
Move std.typetuple to std.meta.list #2687
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
Conversation
Thanks a lot for pushing this! I think I should provide more context where my original attempt to implement DIP54 has stalled. I wanted to provide a bit more fleshed out module before doing the pull request and started with hardest part - overloads based on And since surprise addition to Phobos dev team it has been totally stalled because it eats almost all spare time :( In that regard I am happy someone else looking at this and fully approve attempt to start with smaller and more obvious changes so that at least something can happen that way. On actual naming debate and stuff:
This is not entirely true as function argument list is almost a subset of template argument list and specific argument list provided after
Unfortunately there isn't anything we can do with it realistically so it wasn't mention. Only thing I can imagine is providing Phobos wrapper
I don't think it is a good change. Better way to deprecate the module is to just leave it as-is minimizing any potential behavior changes. |
As for this one:
I was initially going to advocate idiom of doing However, practice has shown that even that long name creates confusion in what it actually means so this rationale doesn't hold. |
Perhaps I should start a thread about TAL naming. I don't mind using full TAL though.
Sounds good.
OK, I'll do that soon. |
96a1a81
to
c151fd3
Compare
Rebased. Now std.typetuple contents are kept, just the deprecation note is added: ntrel@bf83624#diff-2 |
Judging by previous experience that won't result in any agreement :) At least not unless you are going to provide strong proposals of your own, better than existing ones. |
Thank you for moving forward with this ! Some points:
Thanks, it makes it much more easier to review. However, now we can deprecate a whole module, and if we provide a replacement, we should do so. A red warning on the website is nice, but users won't notice it in existing code.
I agree to |
Yes this now looks to me like a better course of actions. Most of
AFAIK yes, it is still relevant. What is exactly the problem? |
A non blocker for this P.R., but as we overhaul this module, I thought we could bring some consistency. I opened a discussion here: dlang/dlang.org#700 |
btw about naming, I am kind of starting to like this proposal by @klickverbot :
|
I don't recall ever seeing anything close to agreement that this level of overhaul of these modules is likely to be accepted. I strongly suggest making sure there's buy-off before investing too much time here. |
It would be very good to get that documented in the DIP and get it out of 'draft' status. I'm sure it's going to produce another round of discussion and controversy. |
Yes; I wasn't sure what the deprecation process is. Do we go straight to deprecation now?
Looks good. Also for std.traits.FooTuple I assume that should go to FooList? |
The first one links to the spec term, but the second one I've now replaced with 'template argument lists', to avoid interpretation of TAL as a symbol name. |
@ntrel: #1766 (comment) is always my reference for deprecation process. So we go right for deprecation (as explained). In addition, in this case, it's a bit special: We are moving a symbol, which means we are not going to undocument it, so this part doesn't apply. |
While that appeals if we were starting from scratch, I think it will complicate the process of porting code to std.meta.list somewhat - users would also have to add I now think using just |
I don't think this is much of a concern. The whole point of |
I agree. This is pretty much "last chance to get it right" thing. |
I will try reaching @andralex and checking on his up to date opinion on topic |
OK, so far we have |
I think that's all the changes local to |
a64aa4f
to
6d639bd
Compare
OK, made a start on this. I'm using I looked at #690 regarding |
I think plain @JakobOvrum are you ok with proposed naming? @ntrel this branch needs a rebase, tester fails because of merge conflicts. |
@Dicebot I agree e.g. for BTW, I chose Also, I wondered about just having |
Mark std.typetuple as deprecated. Changes to std.meta.list contents will be done separately for review.
Set module name = std.meta.list Add example of TypeTuple with non-type arguments Change usage of typetuple -> list Fix using 'type' where non-types are acceptable
It is generally OK but I'd really like to have I will give a line-by-line read through during upcoming week. For now please try fixing auto-tester failures ;) |
OK. I think the auto-tester failures are just due to |
|
Yes. There were no conflicts when I wrote that. Makefile changes will probably keep causing conflicts every few days. |
Back here. I had a look at actual sources and see that it still moves all symbols to
If you refer to unit-tests, there is a feature called |
OK, I hadn't realized. Thanks for the info about deprecated unittests. |
Any updates on this @ntrel ? |
My uncollected thoughts:
So we have:
Are users supposed to remember these rules? We're fixing the problem of conflation with tuples which is the first priority, but I was under the impression that fixing this problem of prefixes and conventions was also on the table, as it too requires "breakage" (users are free to use the old module). The This module adds a bunch of new functionality. The ones I can see: I've dropped any pretense I had of established naming conventions for this overhaul; what's important is that we're consistent from here on out. I'm not going to oppose merging this PR even in its current state, but I do wish we'd actually solve the naming problems instead of just choosing new but equally arcane rules. I'll give it some thought, although maybe the last thing we need is yet another suggestion for how to structure these symbols... |
I'd love it to be that way but it was mentioned as historical mistake and new rule is "no name collisions within Phobos at all". AFAIR specifically @andralex was against adding new name clashes. |
Sorry, I haven't been working on this, I'm not that motivated about this any more. |
No problem, I will take it over from here on with your permission. |
@Dicebot sure, go ahead. |
kk, on it, expect replacement PR as soon as I make decisions on few tricky details. @JakobOvrum regarding |
https://issues.dlang.org/show_bug.cgi?id=4113
Part of DIP 54.
My plan of attack is to create
std.meta.list
as described in the DIP, then work on updating dlang.org and remaining Phobos modules not to use tuple for template argument lists. I don't intend to work onstd.meta.pack
as that seems additional functionality best done by someone else.BTW I think the DIP also needs to mention what happens to
.tupleof
- copying @Dicebot.For the documented new symbol name for TypeTuple, TemplateArgumentList, I think the 'Template' word is redundant because there's always a '!' after the symbol, e.g. TemplateArgumentList!int, so the reader knows it's a template. I suggest we use just ArgumentList
or ArgList. It's still descriptive, and we need a short name (e.g. for easy use in unittests) otherwise everyone will just alias it differently. But I will change this if necessary.std.typetuple
tostd.meta.list
.std.typetuple
.import meta = std.meta.list
(as discussed below).std.meta.list
docs to mention template argument list or just list instead of tuple.std.typetuple
,TypeTuple
in Phobos.