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

Collection initializers and collection expressions doesn't work with Seq<T> #1367

Open
piotr-urbanski opened this issue Sep 19, 2024 · 3 comments
Labels
enhancement v4 Related to version < 5.0.0

Comments

@piotr-urbanski
Copy link

piotr-urbanski commented Sep 19, 2024

Creating Seq with collection initializer or collection expression doesn't work.

[Test]
public void Create_Seq_with_constructor()
{
	Seq<string> seq = new Seq<string>(["a", "b"]);
	Assert.That(seq, Has.Count.EqualTo(2));
}

[Test]
public void Create_Seq_with_collection_initializer()
{
	Seq<string> seq = new Seq<string> { "a", "b" };
	Assert.That(seq, Has.Count.EqualTo(2)); // FAIL
}

[Test]
public void Create_Seq_with_collection_expression()
{
	Seq<string> seq = ["a", "b"];
	Assert.That(seq, Has.Count.EqualTo(2)); // FAIL
}

Finding this error was tricky, because code looks ok and its hard to imagine, that variable will not have the value assigned in literally previous line of code, so I was looking everywhere before I checked if this initialization works. It's a shame they didn't replace the { } constructor route to use the CollectionBuilder if one is available, seems very short sighted to me.

@louthy
Copy link
Owner

louthy commented Sep 19, 2024

Create_Seq_with_constructor and Create_Seq_with_collection_expression work for me.

Could you:

  • Make sure you have the latest version of language-ext (and let me know that you're on it)
  • Run the code below, so I can rule out problems with whatever unit-testing framework you're using. I've noticed that they can a little temperamental with collections.
    public static void Test()
    {
        Create_Seq_with_constructor();
        Create_Seq_with_collection_expression();
    }

    public static void Create_Seq_with_constructor()
    {
        var seq = new Seq<string>(["a", "b"]);
        Debug.Assert(seq.Count == 2);
    }

    public static void Create_Seq_with_collection_expression()
    {
        Seq<string> seq = ["a", "b"];
        Debug.Assert(seq.Count == 2);
    }

Create_Seq_with_collection_initializer will never work because C# just calls .Add and ignores the result, which for immutable collections means the collection never gets any values added. I raised the issue on the dotnet/csharplang repo four years ago, which I assume triggered the new collection-initialiser support.

By the way, before CollectionBuilder, the idiomatic way to create collections is like so:

using static LanguageExt.Prelude;

var xs = Seq(1, 2, 3, 4, 5);
var xs = List(1, 2, 3, 4, 5);
var xs = Set(1, 2, 3, 4, 5);
var xs = HashSet(1, 2, 3, 4, 5);
var xs = Map(("x", 1), ("y, 2), ("z", 3));
var xs = HashMap(("x", 1), ("y, 2), ("z", 3));

It's still usually better than using [1, 2, 3, 4, 5] because it already knows the type.

@piotr-urbanski
Copy link
Author

Thank you for your answer.
Constructor also works for me, but collection expression and collection initializer don't.
When I run the code you posted debugger stops at Debug.Assert in Create_Seq_with_collection_expression.

I'm using the latest stable version 4.4.9:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="LanguageExt.Core" Version="4.4.9" />
  </ItemGroup>
</Project>

And this program:

using LanguageExt;

Test();

void Test()
{
	Create_Seq_with_constructor();
	Create_Seq_with_collection_initializer();
	Create_Seq_with_collection_expression();
}

void Create_Seq_with_constructor()
{
	var seq = new Seq<string>(["a", "b"]);
	Console.Out.WriteLine("Constructor: " + seq.Count);
	Console.Out.WriteLine(seq);
}

void Create_Seq_with_collection_initializer()
{
	Seq<string> seq = new Seq<string> { "a", "b" };
	Console.Out.WriteLine("Collection initializer: " + seq.Count);
	Console.Out.WriteLine(seq);
}

void Create_Seq_with_collection_expression()
{
	Seq<string> seq = ["a", "b"];
	Console.Out.WriteLine("Collection expression: " + seq.Count);
	Console.Out.WriteLine(seq);
}

produces:

Constructor: 2
[a, b]
Collection initializer: 0
[]
Collection expression: 0
[]

According to ReSharper's "IL viewer" collection expression is lowered to collection initializer in High-level C# so I don't understand how it is possible, that one does work for you and the other doesn't.

In Low-level C# they both look the same:

  [CompilerGenerated]
  internal static void <<Main>$>g__Create_Seq_with_collection_initializer|0_2()
  {
    Seq<string> seq1 = new Seq<string>();
    seq1.Add("a");
    seq1.Add("b");
    Seq<string> seq2 = seq1;
    Console.Out.WriteLine(string.Concat("Collection initializer: ", seq2.Count.ToString()));
    Console.Out.WriteLine((object) seq2);
  }

  [CompilerGenerated]
  internal static void <<Main>$>g__Create_Seq_with_collection_expression|0_3()
  {
    Seq<string> seq1 = new Seq<string>();
    seq1.Add("a");
    seq1.Add("b");
    Seq<string> seq2 = seq1;
    Console.Out.WriteLine(string.Concat("Collection expression: ", seq2.Count.ToString()));
    Console.Out.WriteLine((object) seq2);
  }

So I have 2 questions:

  1. What can I do to make collection expressions work for Seq?
  2. Is there a way to disable collection expression and collection initializers for Seq?

@louthy
Copy link
Owner

louthy commented Sep 19, 2024

v4.* doesn't have support for CollectionBuilder, only v5+, which is in beta atm. That's why it worked for me and not for you.

I'm surprised it compiled at all, it really shouldn't!

The only way to disable access to the C# hack that calls Add repeatedly is to build a Roslyn analyser that throws errors when it finds that pattern. There's a PR open that does this, but it needs work, and it's pretty low down in my priority list: #868 - feel free to take it on if you feel it's important.

@louthy louthy added enhancement v4 Related to version < 5.0.0 labels Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement v4 Related to version < 5.0.0
Projects
None yet
Development

No branches or pull requests

2 participants