Improve std.typecons.Tuple implementation #1308
Conversation
| @@ -822,6 +803,21 @@ unittest | |||
| assert(t2[0] == 1 && t2[1] == 2); | |||
| } | |||
| } | |||
| @safe unittest | |||
There was a problem hiding this comment.
Interesting.. I didn't know we can add properties to unittests.
There was a problem hiding this comment.
Yes. It makes it possible to test that functions are pure or @safe or whatnot.
|
Will #1248 be possible to implement after this pull? Perhaps we should try to pull that one before this one? |
|
@AndrejMitrovic This improvement would not break #1248. |
| @@ -268,10 +268,8 @@ Tuple!(int, int) point2; | |||
| assert(!is(typeof(point1) == typeof(point2))); // passes | |||
| ---- | |||
| */ | |||
| struct Tuple(Specs...) | |||
| template Tuple(Specs...) | |||
There was a problem hiding this comment.
why change from struct to template?
There was a problem hiding this comment.
To hide some utility templates and functions by eponymous template trick.
There was a problem hiding this comment.
I still have a question. When I originally proposed allowing additional members with the eponymous trick, I suggested that all other members should be private. It seems the feature works even with public members... why?
There was a problem hiding this comment.
Currently compiler always rewrite Tuple!(...).ident to Tuple!(...).Tuple.ident. The rewriting occurs before checking member access rights, so if eponymous member exists in the template instance, other instance members will be invisible from outside of the instance. Then there's no need adding private attribute to all other members.
There was a problem hiding this comment.
Yah, so I figured. Only access from within works. I'm just a bit bummed that now we have yet another way of hiding things :o).
There was a problem hiding this comment.
AFAIK, it's continuous behavior before the start of my contributing.
The utility templates and functions are not necessary in Tuple struct.
It's automatically forwarded to expand.length through `alias this`.
`w` (==`writer`) is better than `app`(==`appender`).
- Add @trusted attribute - Change parameter types `uint` to `size_t` - Limit the range of fields by template constraint
|
@andralex OK, I fixed all review points. |
|
merging now, still wondering about eponymous/private |
Improve std.typecons.Tuple implementation
I'm wondering about this too. To recap, IIRC you want to allow these: template S()
{
struct S { } // eponymous
struct Priv { } // private
public struct Pub { } // accessible
}
alias A = S!(); // A becomes struct S
alias B = S!().Priv; // disallowed, 'Priv' is private
alias C = S!().Pub; // C becomes 'Pub'Is that it? Because I've actually had this come up in my own code several times, and I thought it was supposed to work. |
|
@AndrejMitrovic template S ()
{
struct S { int Priv; int Pub; } [a]
struct Priv { } // [b]
public struct Pub { } // [c]
}
alias A = S!(); // A becomes struct S
alias B = S!().Priv; // is [a] or [b]? --> currently [a] is chosen
alias C = S!().Pub; // [is [a] or [c]? --> currently [a] is chosen |
This improvement has two purpose.
Remove unnecessary workadounds that was for legacy compiler bugs.
Named-field tuple should be a subtype of unnamed-field tuple.
Required compiler fixes by this:
dlang/dmd#2084(merged)dlang/dmd#2085(merged)