Add sumtype to Phobos

[pbackus]

Based on version 1.0.3 of the sumtype package on code.dlang.org. See individual commit messages for a summary of changes.

[dlang-bot]

Thanks for your pull request and interest in making D better, @pbackus! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + phobos#7702"

[PetarKirov]

@pbackus thank you for your work on sumtype! It's an excellent high-quality module that I've used successfully in various projects, as I'm sure many other D developers have. I believe that adding it to phobos (and later druntime), would be an excellent addition to D's standard distribution!

@atilaneves What's the DLF's latest position on adding new modules to phobos? Is there a checklist from your side about about what a pull request like this one should fulfill?

I see several options:

1. Review and merge this under std.sumtype in master after the 2.095.0-beta.1 cut-off point (expected soon™). We would have close to two months of time until the until dmd 2.096.0-beta.1 (scheduled for 2021-01-15) to make any large API-level changes (obviously non-breaking changes are always possible).
2. If adding a new top-level module is deemed to risky (I don't think so), we can merge it under std.experimental.sumtype after a short review and set a deadline for "stabilizing" after which it will become an "official" phobos module under std.sumtype (e.g. after 2-3 releases).
3. Announce a 2-3 weeks long RFC process on the forums, during which we ask as many members of the community to review, after we vote on a hard yes/no decision and we either merge under std.sumtype, or we restart the RFC process.

[pbackus]

Re: stabilization, my view is that stabilization can and should happen on code.dlang.org. std.experimental seems in practice to have become a kind of purgatory where modules are considered too stable to break, but also not stable enough that users feel confident depending on them. I would not consider it a successful outcome if sumtype were to end up there.

[atilaneves]

@atilaneves What's the DLF's latest position on adding new modules to phobos? Is there a checklist from your side about about what a pull request like this one should fulfill?

Good question. Given that this is the first time it's happened since I became the deputy leader, I'm going to have to think about it.

[Bolpat]

My latest news about adding modules to Phobos is that they need to bring great merit since removing them later is a breaking change.

@pbackus Can you outline why adding this to Phobos is a big win? Especially, why DUB isn't enough?

[pbackus]

@Bolpat I'd be glad to. Here's something I wrote about this on the D forums [2] that I think summarizes the argument pretty well:

In the C++ community, certain types are regarded as "vocabulary types"--types that "provide a single lingua franca, a common language, for dealing with [their] domain." [1] The prototypical example is std::string. What's important about these types is not just that they're available, but that they're standardized. std::string isn't just one possible choice of string class, it's the obvious choice.

Which types benefit the most from this kind of standardization? In broad terms, types that are used for communication between different libraries or modules. These tend to be general-purpose, domain-agnostic types like strings, dynamic arrays, associative arrays, tuples, and--that's right--sum types.

I don't particularly care if SumType or some other implementation is accepted into Phobos (though of course, I prefer SumType myself), but I think it's important for there to be a sum type in D that (1) meets the quality standards of today's D programmers, and (2) is the obvious choice. Currently, SumType satisfies (1), but not (2); and Algebraic satisfies (2), but not (1).

Personally, I don't think it's possible to make a dub package "more obvious" than a standard library module, no matter how many shiny stickers you put on it. So, given that Algebraic is already in Phobos, and likely to remain there for a long time, I think the most sensible path forward is for SumType, or something like it, to be added alongside it.

I would add to this that the reasons to prefer a new module over an in-place upgrade of Algebraic are that (1) we don't have to worry about breaking existing code, and (2) it gives us an opportunity to leave behind the shortcomings of Algebraic's API, such as the default "uninitialized" state, the use of exceptions for error handling, and of course the name Algebraic (I think someone once remarked that naming a discriminated union implementation Algebraic is like naming a hash table implementation Container).

Please feel free to ask if you have any more questions.

[1] https://www.oreilly.com/library/view/mastering-the-c17/9781787126824/c6737dd3-47fa-4719-a459-72afac90857c.xhtml
[2] https://forum.dlang.org/post/kuwarcdbclbhedgxutbw@forum.dlang.org

[RazvanN7]

@atilaneves Have you come to a conclusion regarding this addition?

It is going to require some work to review this. @andralex @wilzbach @jmdavis @CyberShadow @MartinNowak @thewilsonator and everyone else who has the time. Maybe we can coordinate to review parts of this addition?

[atilaneves]

@RazvanN7 No, not yet, sorry. I know I should do that soon.

[pbackus]

@atilaneves Can you give any kind of estimate for when we can expect some progress on this? If not, what would it take to generate such an estimate? I'm happy to help in any way I can, but complete radio silence from you/the DLF means my opportunities to do so are severely limited.

[pbackus]

Kicking things off here with a self-review. Any and all feedback is appreciated, but I'm especially hoping to hear from experienced Phobos developers regarding the build-system changes.

[pbackus]

Not sure if I ought to be re-using this directory from the regular betterC tests or not, but it seems to work.

[pbackus]

Having this information duplicated in two different files is not ideal. One possible alternative would be to put the list in its own file, and have both posix.mak and betterc_module_tests.d read it in via $(file ...) and string import, respectively, but the extra layer of indirection would make each file harder to read in isolation.

[pbackus]

This list is repeated in posix.mak. See the comment there for discussion.

[pbackus]

Splitting this into a new object file is probably not necessary, but I don't have a Windows dev environment set up that I can test in, so I didn't want to risk it.

[pbackus]

See comment in win32.mak.

[pbackus]

The rationale for these disabled checks, copy+pasted from the relevant commit message:

• assert_without_message: flags asserts inside invariants, even though
  their messages are ignored.
• could_be_immutable_check: flags variables inside unittest blocks.
• has_public_examples: flags SumType because its examples are attached
  to the module rather than the SumType symbol itself.
• object_const_check: does not acknowledge a template this parameter as
  a valid alternative to inout on opEquals and toString.
• opequals_tohash_check: fails to detect SumType.toHash.
• properly_documented_functions: requires 'Params' documentation for
  template this parameters and parameters of private (but documented)
  functions.

[atilaneves]

@atilaneves Can you give any kind of estimate for when we can expect some progress on this? If not, what would it take to generate such an estimate? I'm happy to help in any way I can, but complete radio silence from you/the DLF means my opportunities to do so are severely limited.

I'm giving myself until next Wed. Keep me to it.

[pbackus]

@atilaneves Ping. What's the next step here?

[dukc]

Ping @atilaneves

Decision still needed

[andralex]

Ping @atilaneves

Decision still needed

Atila is off for a while for personal reasons. @WalterBright should be able to help in the interim. What is the decision to be taken?

[maxhaton]

Ping @atilaneves
Decision still needed

Atila is off for a while for personal reasons. @WalterBright should be able to help in the interim. What is the decision to be taken?

The decision is whether to merge into Phobos or not.

I think sumtype is of Phobos-quality but the decision is two-fold:

• If we merge should it be in Phobos or core (Not everyone uses Phobos, they now have to copy and paste one module in the worst case) - the library is nogc and betterC so I would vote for core. Putting it druntime also encourages standardisation around it.

• Should we be thinking bigger than this - as good a job as Paul has done here, algebraic data types as-a-library are just not very ergonomic, fast (at least one call to staticMap for every pattern match), and are harder to type check. The lack of ADTs is, in my mind, one of not many things that D really lacks.

[pbackus]

The decision is whether to merge into Phobos or not.

As I read it, there are three questions that need to be answered here:

1. What are the requirements for adding a new module to Phobos?
2. Does sumtype meet those requirements?
3. Assuming that sumtype meets those requirements, do we want to include it in Phobos?

Both 1 and 2 require input from Walter and/or Atila, so progress on those fronts is blocked until one of them is available. The rest of your comment falls under 3, and I will address it in a separate reply.

[pbackus]

If we merge should it be in Phobos or core (Not everyone uses Phobos, they now have to copy and paste one module in the worst case) - the library is nogc and betterC so I would vote for core. Putting it druntime also encourages standardisation around it.

I'm agnostic on this, but I think it's important to mention that sumtype currently depends on Phobos for its toString method. It's possible we'll want to move string formatting into core too, especially if DIP 1036 ends up being accepted, so that's not necessarily an insurmountable obstacle, but it's not quite as simple as just copy+pasting one module.

Should we be thinking bigger than this - as good a job as Paul has done here, algebraic data types as-a-library are just not very ergonomic, fast (at least one call to staticMap for every pattern match), and are harder to type check. The lack of ADTs is, in my mind, one of not many things that D really lacks.

I am not sure it would be wise to give up on useful, working code today in favor of a hypothetical future language feature that hasn't even had a DIP written for it yet, much less an implementation. D's history is littered with ambitious projects that looked promising early on, generated a bunch of excitement, and eventually ended up abandoned. By all means, we should consider our options, but let's be realistic about what those options actually are.

[pbackus]

Auto-tester is currently red because one of the Darwin runners timed out while attempting to build DMD. It should fix itself the next time a new run is triggered. Edit: and indeed it has.

[atilaneves]

I think that all the tests that have assert(__traits(compiles, /* ... */))); should just have whatever should compile due to compiler errors being printed if they don't - otherwise it'll be hard to diagnose why something is failing.

Other than that (minor anyway) and my question just now, should be good to go.

[pbackus]

@atilaneves Should be ready to merge now.

[RazvanN7]

Thanks to @pbackus for this awesome addition and to everyone involved in the review process.