www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - RFC: std.sumtype (adding sumtype to Phobos)

reply Paul Backus <snarwin gmail.com> writes:
Following the recent 1.0.0 release [1], I have decided to submit 
sumtype [2] for inclusion in Phobos, D's standard library. The PR 
can be found here:

https://github.com/dlang/phobos/pull/7702

This thread is for you, the D community, to give your feedback, 
comments, reviews, and constructive criticism regarding both 
sumtype itself, and my proposal to include it in D's standard 
library. Please keep your replies on-topic. If you are unsure 
what to include in your comments, the Boost reviewer guidelines 
[3] have some good advice.

THIS IS NOT A VOTING THREAD. IF YOU WISH TO SHOW 
SUPPORT/OPPOSIION, GIVE THE PR A "THUMBS UP" OR "THUMBS DOWN" ON 
GITHUB.

[1] 
https://forum.dlang.org/post/zagxftjoxxzirneavymj forum.dlang.org
[2] https://code.dlang.org/packages/sumtype
[3] https://www.boost.org/community/reviews.html#Comments
Nov 22 2020
next sibling parent reply rinfz <cherez mailbox.org> writes:
On Sunday, 22 November 2020 at 13:25:37 UTC, Paul Backus wrote:

 Following the recent 1.0.0 release [1], I have decided to 
 submit sumtype [2] for inclusion in Phobos, D's standard 
 library.


How will this impact std.variant?
Nov 22 2020
parent Paul Backus <snarwin gmail.com> writes:
On Sunday, 22 November 2020 at 16:03:02 UTC, rinfz wrote:

 On Sunday, 22 November 2020 at 13:25:37 UTC, Paul Backus wrote:

 Following the recent 1.0.0 release [1], I have decided to 
 submit sumtype [2] for inclusion in Phobos, D's standard 
 library.


 How will this impact std.variant?


There will be no changes to std.variant, except that the 
documentation for Algebraic will be changed to recommend SumType 
as an alternative.
Nov 22 2020
prev sibling next sibling parent reply SHOO <zan77137 nifty.com> writes:
On Sunday, 22 November 2020 at 13:25:37 UTC, Paul Backus wrote:

 Following the recent 1.0.0 release [1], I have decided to 
 submit sumtype [2] for inclusion in Phobos, D's standard 
 library. The PR can be found here:

 https://github.com/dlang/phobos/pull/7702


I think SumType is better to be included in std.typecons. Rather 
than a new module, I think it is more appropriate to treat it in 
the same breath as Tuple, Unique, etc.
Nov 25 2020
parent reply Paul Backus <snarwin gmail.com> writes:
On Wednesday, 25 November 2020 at 15:07:38 UTC, SHOO wrote:

 I think SumType is better to be included in std.typecons. 
 Rather than a new module, I think it is more appropriate to 
 treat it in the same breath as Tuple, Unique, etc.


I considered that as an option. The reasons I decided against it 
were:

1. std.typecons is one of the largest modules in Phobos, at over 
9000 lines. One Phobos maintainer I spoke to mentioned that they 
were already considering splitting it up into separate modules 
like std.tuple, std.nullable, std.unique, etc. Given all that, 
adding sumtype's 2500 lines to the pile didn't seem like a great 
idea.

2. sumtype's unittest suite is compatible with BetterC, but 
std.typecons' is not, so merging the two modules would result in 
worse test coverage (which could lead to regressions in sumtype's 
BetterC compatibility in the future).

3. sumtype's documentation makes extensive use of 
cross-references, and is much easier to follow when it's on its 
own dedicated page rather than mixed up with a whole bunch of 
unrelated stuff.
Nov 25 2020
parent reply Petar Kirov [ZombineDev] <petar.p.kirov gmail.com> writes:
On Wednesday, 25 November 2020 at 15:37:42 UTC, Paul Backus wrote:

 On Wednesday, 25 November 2020 at 15:07:38 UTC, SHOO wrote:

 I think SumType is better to be included in std.typecons. 
 Rather than a new module, I think it is more appropriate to 
 treat it in the same breath as Tuple, Unique, etc.


 I considered that as an option. The reasons I decided against 
 it were:

 1. std.typecons is one of the largest modules in Phobos, at 
 over 9000 lines. One Phobos maintainer I spoke to mentioned 
 that they were already considering splitting it up into 
 separate modules like std.tuple, std.nullable, std.unique, etc. 
 Given all that, adding sumtype's 2500 lines to the pile didn't 
 seem like a great idea.

 2. sumtype's unittest suite is compatible with BetterC, but 
 std.typecons' is not, so merging the two modules would result 
 in worse test coverage (which could lead to regressions in 
 sumtype's BetterC compatibility in the future).

 3. sumtype's documentation makes extensive use of 
 cross-references, and is much easier to follow when it's on its 
 own dedicated page rather than mixed up with a whole bunch of 
 unrelated stuff.


FWIW, I'd prefer not to split `std.typecons` into independent 
modules like `std.tuple`, `std.nullable`, `std.unique`, etc., but 
as as a package containing those modules: `std.typecons.tuple`, 
`std.typecons.nullable`, `std.typecons.unique`. So sumtype could 
continue to leave in a single module, but under the 
`std.typecons` package: `std.typecons.sumtype`. A few years ago 
we split std.algorithm like that and this didn't breaking any 
existing code as far as I remember, as we used `public import`s 
for backward compatibility.
Nov 25 2020
parent reply Paul Backus <snarwin gmail.com> writes:
On Wednesday, 25 November 2020 at 16:02:01 UTC, Petar Kirov 
[ZombineDev] wrote:

 FWIW, I'd prefer not to split `std.typecons` into independent 
 modules like `std.tuple`, `std.nullable`, `std.unique`, etc., 
 but as as a package containing those modules: 
 `std.typecons.tuple`, `std.typecons.nullable`, 
 `std.typecons.unique`. So sumtype could continue to leave in a 
 single module, but under the `std.typecons` package: 
 `std.typecons.sumtype`. A few years ago we split std.algorithm 
 like that and this didn't breaking any existing code as far as 
 I remember, as we used `public import`s for backward 
 compatibility.


I just don't see any reason for the extra level of nesting. The 
module name `std.typecons.tuple` doesn't convey any more 
information than `std.tuple`. Are we saving the name `std.tuple` 
for something else? Are we planning on having multiple `tuple` 
modules in Phobos, such that we'll need to distinguish between 
`std.typecons.tuple` and `std.something_else.tuple`?

We can (and should, of course) use public imports for backward 
compatibility regardless of what we name the new modules.
Nov 25 2020
parent reply Petar Kirov [ZombineDev] <petar.p.kirov gmail.com> writes:
On Wednesday, 25 November 2020 at 16:22:31 UTC, Paul Backus wrote:

 On Wednesday, 25 November 2020 at 16:02:01 UTC, Petar Kirov 
 [ZombineDev] wrote:

 FWIW, I'd prefer not to split `std.typecons` into independent 
 modules like `std.tuple`, `std.nullable`, `std.unique`, etc., 
 but as as a package containing those modules: 
 `std.typecons.tuple`, `std.typecons.nullable`, 
 `std.typecons.unique`. So sumtype could continue to leave in a 
 single module, but under the `std.typecons` package: 
 `std.typecons.sumtype`. A few years ago we split std.algorithm 
 like that and this didn't breaking any existing code as far as 
 I remember, as we used `public import`s for backward 
 compatibility.


 I just don't see any reason for the extra level of nesting. The 
 module name `std.typecons.tuple` doesn't convey any more 
 information than `std.tuple`. Are we saving the name 
 `std.tuple` for something else? Are we planning on having 
 multiple `tuple` modules in Phobos, such that we'll need to 
 distinguish between `std.typecons.tuple` and 
 `std.something_else.tuple`?

 We can (and should, of course) use public imports for backward 
 compatibility regardless of what we name the new modules.


Well, perhaps `tuple` is a clear enough name, and it could stand 
on its own, but what about things like BlackHole? For me the 
reasons for the nesting are:
* (subjective) it looks better organized and aesthetically 
pleasing to put related modules under a single package. Even if I 
would design the module structure from scratch, I'd still put 
`tuple` under `typecons` (or e.g. `algebraic` / `adt`, along with 
`sumtype`).
* After the split, existing users won't have troubles (however 
small) finding the modules as they would remain grouped under 
their original `typecons` parent
* Nothing to deprecate -> no future breaking changes
Nov 25 2020
next sibling parent Adam D. Ruppe <destructionator gmail.com> writes:
On Wednesday, 25 November 2020 at 16:50:53 UTC, Petar Kirov 
[ZombineDev] wrote:

 * After the split, existing users won't have troubles (however 
 small) finding the modules as they would remain grouped under 
 their original `typecons` parent


Eh you can public import anything and just link it together in 
the docs too. My adrdox does it automatically too:

http://dpldocs.info/experimental-docs/std.string.html#public-imports
Nov 25 2020
prev sibling parent Paul Backus <snarwin gmail.com> writes:
On Wednesday, 25 November 2020 at 16:50:53 UTC, Petar Kirov 
[ZombineDev] wrote:

 Well, perhaps `tuple` is a clear enough name, and it could 
 stand on its own, but what about things like BlackHole?


For the parts that don't stand on their own, we can either leave 
them in std.typecons, extract related subsets into new modules as 
a group, or find them new homes in existing modules.


 For me the reasons for the nesting are:
 * (subjective) it looks better organized and aesthetically 
 pleasing to put related modules under a single package. Even if 
 I would design the module structure from scratch, I'd still put 
 `tuple` under `typecons` (or e.g. `algebraic` / `adt`, along 
 with `sumtype`).


I think it *can* be useful to group related modules together in a 
single package, so long as

1) The modules are actually related.
2) The grouping conveys some useful information that a standalone 
module would not.


for example, there is no particular relationship between 
`Nullable` and `BitFlags`. Meanwhile, your hypothetical 
`std.adt.tuple` and `std.adt.sumtype` fail to satisfy criterion 

know that they are ADTs, in which case the package name "adt" 
doesn't tell you anything new, or you don't know what an ADT is, 
in which case the package name "adt" tells you nothing at all.

While I understand that aesthetic judgement is subjective, I 
don't think it's really justifiable to make decisions about a 
widely-used project like a standard library based on gut feeling 
alone. The goal in naming a module (or a type, a function, a 
variable, etc.) is to communicate information to its users, after 
all, not to satisfy our own personal preferences.


 * After the split, existing users won't have troubles (however 
 small) finding the modules as they would remain grouped under 
 their original `typecons` parent


The solution to this is to add links to each publicly-imported 
module to std.typecons' documentation--i.e., the same thing we 
already do for std.range and std.algorithm.


 * Nothing to deprecate -> no future breaking changes


There is no need to deprecate anything in either case. Even if 
std.typecons is completely hollowed out, there is no reason it 
cannot remain present for backwards compatibility indefinitely.
Nov 25 2020
prev sibling parent reply bachmeier <no spam.net> writes:
On Sunday, 22 November 2020 at 13:25:37 UTC, Paul Backus wrote:

 Following the recent 1.0.0 release [1], I have decided to 
 submit sumtype [2] for inclusion in Phobos, D's standard 
 library. The PR can be found here:

 https://github.com/dlang/phobos/pull/7702

 This thread is for you, the D community, to give your feedback, 
 comments, reviews, and constructive criticism regarding both 
 sumtype itself, and my proposal to include it in D's standard 
 library. Please keep your replies on-topic. If you are unsure 
 what to include in your comments, the Boost reviewer guidelines 
 [3] have some good advice.

 THIS IS NOT A VOTING THREAD. IF YOU WISH TO SHOW 
 SUPPORT/OPPOSIION, GIVE THE PR A "THUMBS UP" OR "THUMBS DOWN" 
 ON GITHUB.

 [1] 
 https://forum.dlang.org/post/zagxftjoxxzirneavymj forum.dlang.org
 [2] https://code.dlang.org/packages/sumtype
 [3] https://www.boost.org/community/reviews.html#Comments


The "Basic Usage" examples show functions preceded by "pure  safe 
 nogc nothrow". Is that required? If so, do we want to go down 
the path of adding that kind of verbosity/complexity to Phobos 
libraries? That would be a substantial change from what we have 
now. It would definitely make the language less appealing 
(particularly with two starting with   and two not).
Nov 25 2020
next sibling parent Petar Kirov [ZombineDev] <petar.p.kirov gmail.com> writes:
On Wednesday, 25 November 2020 at 15:50:47 UTC, bachmeier wrote:

 The "Basic Usage" examples show functions preceded by "pure 
  safe  nogc nothrow". Is that required? If so, do we want to go 
 down the path of adding that kind of verbosity/complexity to 
 Phobos libraries? That would be a substantial change from what 
 we have now. It would definitely make the language less 
 appealing (particularly with two starting with   and two not).


IMO, attributes like that should remain in the source code (e.g. 
the `unittest` blocks from which the docs are built), but not 
shown in the web page, unless the user hovers over the functions. 
IIRC, we already do that for template contraints.

Additionally, template functions which don't have any attributes, 
but have corresponding `unittest`s with attributes, should be 
displayed in the docs as `pure`-enabled ` safe`-enabled or 
something along those lines. That's somewhat related to the ADA 
(Argument Dependent Attribute set) proposal that Mathias 
presented at this DConf [1].

[1]: https://youtu.be/9lOtOtiwXY4?t=946
Nov 25 2020
prev sibling parent reply Paul Backus <snarwin gmail.com> writes:
On Wednesday, 25 November 2020 at 15:50:47 UTC, bachmeier wrote:

 The "Basic Usage" examples show functions preceded by "pure 
  safe  nogc nothrow". Is that required? If so, do we want to go 
 down the path of adding that kind of verbosity/complexity to 
 Phobos libraries? That would be a substantial change from what 
 we have now. It would definitely make the language less 
 appealing (particularly with two starting with   and two not).


No, the attributes are not required. They're just there to show 
that SumType *can* be used in  safe/pure/nothrow/ nogc code, if 
desired. Maybe it's better to leave them off, though, if they're 
distracting readers from the main topic of the example.
Nov 25 2020
parent reply bachmeier <no spam.net> writes:
On Wednesday, 25 November 2020 at 16:12:14 UTC, Paul Backus wrote:

 On Wednesday, 25 November 2020 at 15:50:47 UTC, bachmeier wrote:

 The "Basic Usage" examples show functions preceded by "pure 
  safe  nogc nothrow". Is that required? If so, do we want to 
 go down the path of adding that kind of verbosity/complexity 
 to Phobos libraries? That would be a substantial change from 
 what we have now. It would definitely make the language less 
 appealing (particularly with two starting with   and two not).


 No, the attributes are not required. They're just there to show 
 that SumType *can* be used in  safe/pure/nothrow/ nogc code, if 
 desired. Maybe it's better to leave them off, though, if 
 they're distracting readers from the main topic of the example.


If that's the case, I agree that it's a good thing to show. You 
might consider having a clean version of toFahrenheit and then 
something like

// SumType can be used in  safe/pure/nothrow/ nogc code
pure  safe  nogc nothrow
void freeze(ref Temperature t)

Whatever you do with that, it clears up my concern and I would 
like to see this added to Phobos.
Nov 25 2020
parent reply jmh530 <john.michael.hall gmail.com> writes:
On Wednesday, 25 November 2020 at 17:53:41 UTC, bachmeier wrote:

 [snip]

 If that's the case, I agree that it's a good thing to show. You 
 might consider having a clean version of toFahrenheit and then 
 something like

 // SumType can be used in  safe/pure/nothrow/ nogc code
 pure  safe  nogc nothrow
 void freeze(ref Temperature t)

 Whatever you do with that, it clears up my concern and I would 
 like to see this added to Phobos.


In general, if you can call a function in a block with all the 
attributes, then you can call it without them all too, but that 
may not be known to someone who has not made much use of them.
Nov 25 2020
next sibling parent Paul Backus <snarwin gmail.com> writes:
On Wednesday, 25 November 2020 at 18:42:09 UTC, jmh530 wrote:

 In general, if you can call a function in a block with all the 
 attributes, then you can call it without them all too, but that 
 may not be known to someone who has not made much use of them.


D is a large enough language that most D programmers have 
significant gaps in their language knowledge (e.g., for me, it's 
the details of D's object system). So when you write 
documentation, you can't assume your readers will understand what 
things mean just by looking at them; you have to do a bit of 
hand-holding.
Nov 25 2020
prev sibling parent bachmeier <no spam.net> writes:
On Wednesday, 25 November 2020 at 18:42:09 UTC, jmh530 wrote:

 On Wednesday, 25 November 2020 at 17:53:41 UTC, bachmeier wrote:

 [snip]

 If that's the case, I agree that it's a good thing to show. 
 You might consider having a clean version of toFahrenheit and 
 then something like

 // SumType can be used in  safe/pure/nothrow/ nogc code
 pure  safe  nogc nothrow
 void freeze(ref Temperature t)

 Whatever you do with that, it clears up my concern and I would 
 like to see this added to Phobos.


 In general, if you can call a function in a block with all the 
 attributes, then you can call it without them all too, but that 
 may not be known to someone who has not made much use of them.


I didn't know that was always the case, but definitely qualify as 
someone that does not add attributes to his functions.
Nov 25 2020