Nim icon indicating copy to clipboard operation
Nim copied to clipboard

add `std/optionals`; deprecate `std/options`; alternative to #18401

Open ringabout opened this issue 2 years ago • 14 comments

ref https://github.com/nim-lang/Nim/pull/18401#issuecomment-1119492386

ringabout avatar May 25 '22 08:05 ringabout

Instead of deprecating a module that is used by (I am guessing) most Nim code, how about we instead make this a breaking change for Nim v2?

@dom96 see also the alternative pr https://github.com/nim-lang/Nim/pull/18401

ringabout avatar May 25 '22 14:05 ringabout

breaking change for Nim v2?

For those of us that have significant codebases in Nim, changes to language and standard library changes that frivolously break our code and assumptions are deeply problematic - every time it happens, it represents a significant cost and constitutes a signal to other organizations to not make similar investments.

Additive changes are the way to go.

arnetheduck avatar May 25 '22 15:05 arnetheduck

@arnetheduck yes, breaking changes are tough. But they are necessary eventually. Perhaps 2.0 is coming too soon, but do you agree that Nim eventually needs to evolve and leave its old baggage behind?

Ideally we would keep Nim 1.x going and support it way past 2.0 to show that those who have invested in that ecosystem are not being left behind.

dom96 avatar May 25 '22 16:05 dom96

Version 2.0 or not, if we can avoid breaking code we will.

Araq avatar May 25 '22 19:05 Araq

Wouldn't it be better to have 2 separate types in the options module? Or is this just a module rename that happens to justify one behavior more?

metagn avatar May 25 '22 22:05 metagn

but do you agree that Nim eventually needs to evolve and leave its old baggage behind?

In the vast majority of cases that have been suggested so far, no - they have been OCD-style "it should be this way because I like the name better or dislike the existing name" and not fundamental.

Where fundamental breaking changes are necessary we've established a way forward for those as well, and not limited to 2.0 (ie provide a better alternative for a period of time, switch default, the finally remove) - the impetus for 2.0 is that it comes with a new memory manager which requires people to learn a new style of code because "canonical and well-written Nim" no longer looks the same similar to when C++11 came along and changed the C++ landscape and how C++ is written.

Notably though, C++ introduced very few actual breaking changes in that transition - there were a few but mostly, the changes were done in an additive manner, which is entirely possible for the vast majority of suggested changes, if you understand the concept that every breaking change decreases the amount of actually working Nim code out there - if adoption is the goal, a vibrant ecosystem of libraries is necessary, and breaking the existing ones is not a good way to get there, really.

The difficult to introduce change and maintain compatibility at the same time is a direct outcome of having a large standard library - for the ecosystem around Nim, this is not a problem really: we can release a new better version and that's it. Nim itself, and its library however, is unique in its ability to break everything out there, sitting at the root of every dependency tree and therefore, it pays of to be more careful.

arnetheduck avatar May 26 '22 09:05 arnetheduck

To elaborate, when upgrading a large codebase, it is often not possible to upgrade everything at once: different libraries have different schedules, requirements, manpower availability and so on - this basically means that if a project is composed of 20 libraries, its not feasible that all of these libraries adapt to the change at the same time - rather, it's a gradual process where both the new and the old world needs to be supported by each library, and when all are ready, the switch can be made.

A breaking change to Nim makes it impossible to perform this incremental transition - it took python 15 years of debate and pain - I don't think it's in Nim's interest to maintain this kind of transition as it is onerous for everyone involved - using techniques like those described (additive changes, or overlapping compatibility periods etc), change can happen without the drama.

arnetheduck avatar May 26 '22 09:05 arnetheduck

the impetus for 2.0 is that it comes with a new memory manager which requires people to learn a new style of code because "canonical and well-written Nim" no longer looks the same

That may be true, but as I understand it orc was built to support all existing code without breakage. So making it default shouldn't break any existing code. Because of that wouldn't a 1.8 release be more appropriate? rather than a 2.0 that signals significant breaking changes to Nim?

I do take the point that we should provide transition periods, from that perspective creating a new module and hinting to developers to transition to it is wise (via deprecation). My only concern is the threshold at which we create such new modules, if we do so for any small breaking change then we will soon end up with a range of duplicate modules, something which for Nim developers (especially newcomers and those that cannot pay attention to all the changes) will be tough to deal with.

Thinking about it some more: maybe a better approach in this case and other similar cases would be to introduce an opt-in flag with a warning to transition to it, instead of duplicating the module. Hm, but then we have the problem that the flag applies to the full application, not just a specific package. Just thinking out loud :)

dom96 avatar May 26 '22 11:05 dom96

This needs to be implemented differently. The impl must use a case object and FP additions like flatMap and map etc should not be in the module. It should only have the essentials, not the bloat.

Araq avatar Jun 13 '22 09:06 Araq

FP additions like flatMap and map etc should not be in the module. It should only have the essentials, not the bloat.

IMO those are the minimum functions for Optional[T] to be useful (we can argue about filter, but at least map and flatMap are really useful in my experience). It's only a few function, can you really call that bloat? Most people using optionals will probably want those functions.

konsumlamm avatar Jun 13 '22 10:06 konsumlamm

I've had a little-thought-out, probably unproductive opinion about this for a while and I've decided this being tangentially brought up in a forum post just now is a good enough excuse to say it. I don't think we need an object form of (T, bool) in the standard library. I like having a type that behaves as a wrapper around the validity of existing types. I don't know what the appeal is to having some(nil) and why people are fighting for it, nil used like that just does not make sense to me.

One could say we can wait for not nil to work so we can optimize for it. We can also add stuff like using out-of-bounds values for range or enum types to mean None in both situations. But while we don't have not nil, I much prefer stuff like this that is generally usable and provides at least runtime consistency. Not to mention people can use cast and still "break monad laws" which you can dismiss by saying "cast is unsafe", but safe type conversions from invalid values and trying to use an invalid value in the option will give the same runtime errors.

Calling it "Option" or attaching FP to it is definitely misleading in this case though. And I don't think it's worth the effort of coming up with new optics for it. So I am fine with conceding and making Options centered around FP, however I don't think this will result in much adoption in the standard library.

metagn avatar Sep 05 '22 13:09 metagn

I don't think we need an object form of (T, bool) in the standard library.

This isn't about having a (T, bool) specifically, this is just how it happens to be implemented. It might as well be an object variant like this:

type
  Optional[T] = object
    case has: bool
    of true:
      val: T
    of false:
      discard

I like having a type that behaves as a wrapper around the validity of existing types. I don't know what the appeal is to having some(nil) and why people are fighting for it, nil used like that just does not make sense to me.

A common use case for an optional type is as a return type for some value that may or may not exist. If "the element doesn't exist" is indicated by nil and a nil is returned, it is ambiguous wether the element is present and is nil (some(nil)) or it isn't present (none()).

Not to mention people can use cast and still "break monad laws" which you can dismiss by saying "cast is unsafe", but safe type conversions from invalid values and trying to use an invalid value in the option will give the same runtime errors.

I don't see how cast is relevant for this specifically, it can be used to break any abstraction, does that mean we shouldn't use any abstractions anymore since people can break them with cast?

Calling it "Option" or attaching FP to it is definitely misleading in this case though. And I don't think it's worth the effort of coming up with new optics for it.

Why is "Option" misleading? That's exactly what it is and it works just like in FP.

So I am fine with conceding and making Options centered around FP, however I don't think this will result in much adoption in the standard library.

I don't think there's any evidence for this, there have been several times where people have wanted to use optional types in the stdlib, e.g. #18359.

konsumlamm avatar Sep 05 '22 15:09 konsumlamm

I'd second that providing map/flatMap proc's is useful since Nim doesn't provide a if let Some(x) = val style check or the walrus operator. Those are kinda ugly anyways imho, but map let you do something like:

table1.get("name").map() do(x: int):
  echo "x: ", x

Not perfect but fits in with regular sort/map/etc like in the do notation docs.

elcritch avatar Sep 06 '22 02:09 elcritch

It seems like the old options will stick around. Could a new module possibly resolve the nil issue and provide a result type? I sorta liked what @arnetheduck mentioned https://github.com/nim-lang/Nim/pull/18401#issuecomment-1119496738 regarding Opt[T] being a special case of a Result[T, E] type. That'd also give a bit of a more generic purpose aside from just changing nil ptr behavior? Providing a core Result[T, E] might make it easier to combine libraries using Result types. I hope this hasn't been discussed elsewhere, I'm on my tablet and haven't searched the RFC's.

elcritch avatar Sep 06 '22 02:09 elcritch