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

Add array range support #179

Merged
merged 3 commits into from
Apr 28, 2024
Merged

Add array range support #179

merged 3 commits into from
Apr 28, 2024

Conversation

gao-artur
Copy link
Contributor

Fixes #178

@SimonCropp I tried adding RuntimeHelpers.GetSubArray (the pre .NET 9 version) to support array range, but for some reason, I'm getting a compiler error
image

@SimonCropp
Copy link
Owner

looks like a bug in roslyn. can you try rains a bug in https://github.com/dotnet/roslyn

@gao-artur
Copy link
Contributor Author

Honestly, I have no idea how to describe this bug. I'm not sure they will love the reference to this PR as a repro...

@gao-artur
Copy link
Contributor Author

This is the exception, and it's thrown by all the projects with <Import Project="$(SolutionDir)\TestIncludes.targets" />

System.NullReferenceException: Object reference not set to an instance of an object.
at Microsoft.CodeAnalysis.CodeGen.ReferenceDependencyWalker.VisitSignature(ISignature signature, EmitContext context)
at Microsoft.CodeAnalysis.Emit.CommonPEModuleBuilder.GetFakeSymbolTokenForIL(ISignature symbol, SyntaxNode syntaxNode, DiagnosticBag diagnostics)
at Microsoft.CodeAnalysis.CodeGen.ILBuilder.EmitArrayBlockInitializer(ImmutableArray1 data, SyntaxNode syntaxNode, DiagnosticBag diagnostics) at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.<TryEmitOptimizedReadonlySpanCreation>g__tryEmitAsCachedArrayFromBlob|88_1(NamedTypeSymbol spanType, BoundExpression wrappedExpression, Int32 elementCount, ImmutableArray1 data, ArrayTypeSymbol& arrayType, TypeSymbol elementType)
at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.TryEmitOptimizedReadonlySpanCreation(NamedTypeSymbol spanType, BoundExpression wrappedExpression, Boolean used, BoundExpression inPlaceTarget, Boolean& avoidInPlace, BoundExpression start, BoundExpression length)
at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.TryEmitOptimizedReadonlySpan(BoundObjectCreationExpression expression, Boolean used, BoundExpression inPlaceTarget, Boolean& avoidInPlace)
at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.EmitObjectCreationExpression(BoundObjectCreationExpression expression, Boolean used)
at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.EmitExpression(BoundExpression expression, Boolean used)
at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.EmitArguments(ImmutableArray1 arguments, ImmutableArray1 parameters, ImmutableArray1 argRefKindsOpt) at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.EmitStaticCallExpression(BoundCall call, UseKind useKind) at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.EmitExpression(BoundExpression expression, Boolean used) at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.EmitSequenceExpression(BoundSequence sequence, Boolean used) at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.EmitExpressionCoreWithStackGuard(BoundExpression expression, Boolean used) at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.EmitExpression(BoundExpression expression, Boolean used) at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.EmitStatement(BoundStatement statement) at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.EmitStatementAndCountInstructions(BoundStatement statement) at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.EmitSequencePointStatement(BoundSequencePoint node) at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.EmitStatement(BoundStatement statement) at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.EmitStatements(ImmutableArray1 statements)
at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.EmitUninstrumentedBlock(BoundBlock block)
at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.EmitStatement(BoundStatement statement)
at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.EmitStatements(ImmutableArray1 statements) at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.EmitUninstrumentedBlock(BoundBlock block) at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.EmitStatement(BoundStatement statement) at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.EmitStatementList(BoundStatementList list) at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.EmitStatement(BoundStatement statement) at Microsoft.CodeAnalysis.CSharp.CodeGen.CodeGenerator.GenerateImpl() at Microsoft.CodeAnalysis.CSharp.MethodCompiler.GenerateMethodBody(PEModuleBuilder moduleBuilder, MethodSymbol method, Int32 methodOrdinal, BoundStatement block, ImmutableArray1 lambdaDebugInfo, ImmutableArray1 orderedLambdaRuntimeRudeEdits, ImmutableArray1 closureDebugInfo, ImmutableArray1 stateMachineStateDebugInfos, StateMachineTypeSymbol stateMachineTypeOpt, VariableSlotAllocator variableSlotAllocatorOpt, BindingDiagnosticBag diagnostics, DebugDocumentProvider debugDocumentProvider, ImportChain importChainOpt, Boolean emittingPdb, ImmutableArray1 codeCoverageSpans, AsyncForwardEntryPoint entryPointOpt)
at Microsoft.CodeAnalysis.CSharp.MethodCompiler.CompileMethod(MethodSymbol methodSymbol, Int32 methodOrdinal, ProcessedFieldInitializers& processedInitializers, SynthesizedSubmissionFields previousSubmissionFields, TypeCompilationState compilationState)
at Microsoft.CodeAnalysis.CSharp.MethodCompiler.CompileNamedType(NamedTypeSymbol containingType)
at Microsoft.CodeAnalysis.CSharp.MethodCompiler.<>c__DisplayClass25_0.b__0()

@SimonCropp
Copy link
Owner

Honestly, I have no idea how to describe this bug. I'm not sure they will love the reference to this PR as a repro...

then here is a minimal repro https://github.com/SimonCropp/RuntimeHelpersRepro

/// <param name="fldHandle">A field handle that specifies the location of the data used to initialize the array</param>
[Link("https://learn.microsoft.com/en-us/dotnet/api/system.runtime.compilerservices.runtimehelpers.initializearray")]
[Intrinsic]
public static extern void InitializeArray(Array array, RuntimeFieldHandle fldHandle);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SimonCropp, after adding this method, everything compiles and tests are green. But I have a few questions:

  1. This method should already exist in all the TFMs. Can it cause runtime errors?
    image

  2. While all tests run fine, the ReSharper runner shows the following errors. Any idea how to fix them? Is it safe to ignore?
    image

  3. I created 2 partial RuntimeHelpers classes, as InitializeArray is not necessarily related to the IndexRange feature and theoretically can be required by other features added in the future. Is it ok, or do you prefer a single class?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try this

// <auto-generated />

#if (NET46X && FeatureValueTuple) || NET47X || NET48X || NETSTANDARD2_0 || NETCOREAPP2X

namespace System.Runtime.CompilerServices;

using System.Reflection;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using Link = System.ComponentModel.DescriptionAttribute;

/// <summary>
/// Provides a set of static methods and properties that provide support for compilers.
/// This class cannot be inherited.
/// </summary>
[ExcludeFromCodeCoverage]
[DebuggerNonUserCode]
#if PolyPublic
public
#endif
static partial class RuntimeHelpers
{
    static readonly Action<Array,RuntimeFieldHandle> initializeArrayAction;

    static RuntimeHelpers()
    {
        //CompilationRelaxations is co located with RuntimeHelpers
        var runtimeHelpersType = typeof(CompilationRelaxations)
            .Assembly
            .GetType("System.Runtime.CompilerServices.RuntimeHelpers");
        var initializeArrayMethod = runtimeHelpersType.GetMethod(
            "InitializeArray",
            BindingFlags.Static | BindingFlags.Public,
            null,
            [typeof(Array), typeof(RuntimeFieldHandle)],
            null)!;
        initializeArrayAction = (Action<Array,RuntimeFieldHandle>)initializeArrayMethod.CreateDelegate(typeof(Action<Array,RuntimeFieldHandle>));
    }

    /// <summary>
    /// Provides a fast way to initialize an array from data that is stored in a module.
    /// </summary>
    /// <param name="array">The array to be initialized.</param>
    /// <param name="fldHandle">A field handle that specifies the location of the data used to initialize the array</param>
    [Link("https://learn.microsoft.com/en-us/dotnet/api/system.runtime.compilerservices.runtimehelpers.initializearray")]
    public static void InitializeArray(Array array, RuntimeFieldHandle fldHandle) =>
        initializeArrayAction(array, fldHandle);
}
#endif

and then we dont need IntrinsicAttribute


Array.Copy(array, offset, dest, 0, length);

return dest;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gao-artur gao-artur marked this pull request as ready for review April 27, 2024 20:27
@SimonCropp
Copy link
Owner

ok. going to merge this one. i will clean up the last remaining things after. thanks for all the research

@SimonCropp SimonCropp merged commit 5b90ff1 into SimonCropp:main Apr 28, 2024
1 check passed
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

Successfully merging this pull request may close these issues.

Range operator doesn't work with arrays
2 participants