Skip to content
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

Use struct Tuples #53

Closed
gusty opened this issue Dec 18, 2017 · 10 comments
Closed

Use struct Tuples #53

gusty opened this issue Dec 18, 2017 · 10 comments

Comments

@gusty
Copy link
Member

gusty commented Dec 18, 2017

  • New overloads for struct tuples can be added.
  • They can also be used internally since they offer better performance in some scenarios.

In principle it looks like it won't be a breaking change.

@gusty
Copy link
Member Author

gusty commented Dec 18, 2017

I did a quick test and I confirm it won't break anything, as long as we don't use it everywhere, namely in functions like zip, unzip.

My feeling is that we should add it as an addition, not as a replacement, since there is not only existing code, also F# functions in the core that still use reference tuples, so at this time it's not clear that struct tuples will be adopted by default in the F# ecosystem.

If they do so, it will be a breaking change, and so we'll follow that breaking change here.

The situation is different with the result type because at the moment there is only one function in core that uses the Choice type, which is Async.Catch.

Another thing to decide is whether we should upgrade to NetFramework 4.7 or not, because otherwise System.ValueTuple.dll will have to be added as a Nuget and it will complicate the dependencies. For Netstandard 2.0 it shouldn't be a problem as it is already included.

Still it should be possible to multi-target older versions of the framework, but it's work, and at the moment I don't feel comfortable doing it myself, but as always PRs are welcome and will be accepted as soon as they demonstrate they don't break anything.

@gusty gusty added this to Done in Roadmap V1.0 Jan 21, 2018
@gusty gusty moved this from Done to In progress in Roadmap V1.0 Jan 21, 2018
@wallymathieu
Copy link
Member

So the implication is mainly in how overloading is handled?

@gusty
Copy link
Member Author

gusty commented Jan 24, 2018

No, I think for stuff exposed externally we should keep using ref tuples, that will be the F# way for some time at least. It's mostly internally where we'll use it. And yes, there will be some overloads for monoids and so on, but that shouldn't be an issue (I hope).

The key thing is whether to upgrade to .NET Framework 4.7.

@gusty
Copy link
Member Author

gusty commented Mar 4, 2018

I'm bit unsure about what to do with this.

I haven't seen wide adoption for struct tuples in general.

There is a breaking change regarding tuples in the next F# 4.1 version, I just realized it will break some stuff here, so I prepared a fix.

Maybe we can postpone struct tuples until:

  • Next version of F# is released with the above mentioned breaking change
  • We migrate this project to .Net Framework 4.7
  • People start adopting struct tuples, at the moment nobody complained about it.

and this will be after v1.0 so we would have to do it in a backwards compatibility way.

Any thoughts?

@wallymathieu
Copy link
Member

What breaks? Will it interop with old style tuples?

@gusty
Copy link
Member Author

gusty commented Mar 4, 2018

The breaking change is that next F# version will have .ItemX member property available for syntactic tuples, so by calling for instance item1 (1,2,3) would result in an ambiguous overload resolution error, because now there will be two overloads that match.

The fix I did is straight forward, it removes the overload for System Tuple (the one with the trait call) at the cost of excluding System Tuples but they will become available again when you upgrade to the latest F# version.

@wallymathieu
Copy link
Member

So, better to document that you need to use the latest f# version instead?

@gusty
Copy link
Member Author

gusty commented Mar 4, 2018

Actually it's also possible to fix it in such a way that it will keep working with System Tuples in current F# versions, but it will complicate a lot the code and System Tuples are rarely used, less in a generic way.

In the coming F# version both tuple types will become (almost) equivalents.

We can state in the docs for the moment that item1 works only with F# syntactic tuples.

@wallymathieu
Copy link
Member

Sounds good.

@gusty gusty removed this from In progress in Roadmap V1.0 Mar 6, 2018
@gusty
Copy link
Member Author

gusty commented Nov 26, 2023

Implemented in #524 no breaking changes AFAIK.
Also note about my comments above that at the moment struct tuples don't implement ItemX members.

@gusty gusty closed this as completed Nov 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants