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

Fails to get subarray #8

Closed
kzu opened this issue Oct 24, 2022 · 3 comments
Closed

Fails to get subarray #8

kzu opened this issue Oct 24, 2022 · 3 comments
Labels
wontfix This will not be worked on

Comments

@kzu
Copy link
Contributor

kzu commented Oct 24, 2022

Thanks for the project, looking forward its evolution!

Reproduction Steps

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>netstandard2.0</TargetFramework>
    <LangVersion>Latest</LangVersion>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="PolySharp" Pack="false" Version="1.3.0" />
  </ItemGroup>

</Project>
namespace Poly;

internal class Class1
{
    readonly string[] ns = "Foo.Bar.Baz".Split('.')[..^1];
}

This works with PolyFill.NET but not
PolyShary. Build error is:

Class1.cs(9,53,9,57): error CS0656: Missing compiler required member 'System.Runtime.CompilerServices.RuntimeHelpers.GetSubArray'

image

@Sergio0694
Copy link
Owner

This is expected. Slicing on arrays needs RuntimeHelpers.GetSubArray, which can't be polyfilled because the type already exists (so one can't just "generate one more API" for it). You can use index and slices on all types that expose a Slice method (eg. Span<T> and ReadOnlySpan<T>), and I think you might also be able to define your own slice extension for the compiler to pick up. But this is not polyfilled by design. I will also add, you should not really slice over arrays anyway most of the time, as that'd add more allocations than needed 🙂

For your specific example, you can just update that to:

using System;

internal class Class1
{
    readonly string[] ns = "Foo.Bar.Baz".Split('.').AsSpan()[..^1].ToArray();
}

You'll want the System.Memory package in case you don't have Span<T> 👍

@Sergio0694 Sergio0694 closed this as not planned Won't fix, can't repro, duplicate, stale Oct 24, 2022
@Sergio0694 Sergio0694 added the wontfix This will not be worked on label Oct 24, 2022
@Sergio0694
Copy link
Owner

I will also add, the reason that works with PolyFill.NET is because they're shadowing the existing RuntimeHelpers type. But that is not something I'd recommend doing, because RuntimeHelpers is a class that might very well be needed, and doing so would mean you'd then have to use aliases to use it, which might not even be sufficient given the file is generated in your own assembly.

@kzu
Copy link
Contributor Author

kzu commented Oct 24, 2022

The argument for "you should not really slice over arrays anyway most of the time" is a bit weak, I'd say, provided it's a feature supported in .NET :).

I'd say emitting the relevant extension method so that this scenario Just Works would be pretty nice. Otherwise, you now have to either document a known limitation or provide a code analyzer that will flag this at compile time (better), so we don't have to go fishing on why something that (we're told) should Just Work, actually doesn't (in "some" cases).

But I'm happy to change my code, for sure. I just think it's a better experience if this is supported OOB in whichever way is deemed "better".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants