Implement RuntimeHelpers.SizeOf#100618

jkoritzinsky · 2024-04-03T23:10:01Z

Implement the SizeOf API with the same behavior as the sizeof IL instruction.

ghost · 2024-04-03T23:10:07Z

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

...em.Runtime/tests/System.Runtime.Tests/System/Runtime/CompilerServices/RuntimeHelpersTests.cs

src/coreclr/vm/reflectioninvocation.cpp

...coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs

...veaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.NativeAot.cs

src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.Mono.cs

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

...em.Runtime/tests/System.Runtime.Tests/System/Runtime/CompilerServices/RuntimeHelpersTests.cs

…/Runtime/CompilerServices/RuntimeHelpersTests.cs Co-authored-by: Jan Kotas <jkotas@microsoft.com>

src/coreclr/vm/reflectioninvocation.cpp

…atform

...coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs

...em.Runtime/tests/System.Runtime.Tests/System/Runtime/CompilerServices/RuntimeHelpersTests.cs

...veaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.NativeAot.cs

...coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

jkotas · 2024-04-05T18:59:59Z

...coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs

+        /// <returns>The size of instances of the type.</returns>
+        /// <exception cref="ArgumentException">The passed-in type is not a valid type to get the size of.</exception>
+        /// <remarks>
+        /// This API has the same behavior as if you were to use the IL sizeof instruction with the passed in type as the operand.


Suggested change

/// This API has the same behavior as if you were to use the IL sizeof instruction with the passed in type as the operand.

/// This API returns the same value as the IL sizeof instruction with the passed in type as the operand.

Nit: It does not have the same behavior for error cases.

Or maybe even link to https://learn.microsoft.com/en-us/dotnet/api/system.runtime.compilerservices.unsafe.sizeof that has more details.

# Conflicts: # src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs # src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.NativeAot.cs # src/libraries/System.Runtime/tests/System.Runtime.Tests/System/Runtime/CompilerServices/RuntimeHelpersTests.cs # src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.Mono.cs # src/mono/mono/metadata/icall.c

am11 · 2024-04-08T17:59:29Z

src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.Mono.cs

+
+            Type typeObj = Type.GetTypeFromHandle(type)!;
+            if (typeObj.ContainsGenericParameters || typeObj.IsGenericParameter || typeObj == typeof(void))
+                throw new ArgumentException(SR.Arg_TypeNotSupported);


Is this a limitation of mono runtime (which doesn't exist in coreclr)?

No, this is the same limitation in CoreCLR and NativeAOT (though the implementations are slightly different due to how the different type systems are implemented).

jkoritzinsky · 2024-04-09T19:53:35Z

/ba-g Slow mac timeout

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

jkoritzinsky added 2 commits April 3, 2024 20:56

Implement RuntimeHelpers.SizeOf

3362c4a

Add tests and add various fixes.

47c6a24

jkoritzinsky added the area-System.Runtime label Apr 3, 2024

jkoritzinsky requested review from MichalStrehovsky, lambdageek, marek-safar and thaystg as code owners April 3, 2024 23:10

ghost added the new-api-needs-documentation label Apr 3, 2024

dotnet-policy-service bot assigned jkoritzinsky Apr 3, 2024

jkotas reviewed Apr 3, 2024

View reviewed changes

...em.Runtime/tests/System.Runtime.Tests/System/Runtime/CompilerServices/RuntimeHelpersTests.cs Outdated Show resolved Hide resolved

jkotas reviewed Apr 3, 2024

View reviewed changes

src/coreclr/vm/reflectioninvocation.cpp Outdated Show resolved Hide resolved

PR feedback

16ea830

jkotas reviewed Apr 3, 2024

View reviewed changes

src/coreclr/vm/reflectioninvocation.cpp Outdated Show resolved Hide resolved

jkotas reviewed Apr 3, 2024

View reviewed changes

Apply suggestions from code review

cb7ec92

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

jkotas reviewed Apr 4, 2024

View reviewed changes

...em.Runtime/tests/System.Runtime.Tests/System/Runtime/CompilerServices/RuntimeHelpersTests.cs Show resolved Hide resolved

jkotas reviewed Apr 4, 2024

View reviewed changes

...em.Runtime/tests/System.Runtime.Tests/System/Runtime/CompilerServices/RuntimeHelpersTests.cs Outdated Show resolved Hide resolved

Update src/libraries/System.Runtime/tests/System.Runtime.Tests/System…

47d3b54

…/Runtime/CompilerServices/RuntimeHelpersTests.cs Co-authored-by: Jan Kotas <jkotas@microsoft.com>

jkoritzinsky added 2 commits April 4, 2024 12:25

Add more tests and throw for more invalid cases.

c086418

Generic parameter types don't have MethodTables in NativeAOT.

84b4c0f

jkotas reviewed Apr 4, 2024

View reviewed changes

src/coreclr/vm/reflectioninvocation.cpp Outdated Show resolved Hide resolved

Update src/coreclr/vm/reflectioninvocation.cpp

49e6521

jkotas approved these changes Apr 4, 2024

View reviewed changes

jkoritzinsky added 2 commits April 4, 2024 16:17

Fix CoreCLR

f93baee

Separate out case to skip on NativeAOT as it's not relavant on the pl…

6898911

…atform

jkotas reviewed Apr 4, 2024

View reviewed changes

...coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs Outdated Show resolved Hide resolved

jkotas reviewed Apr 4, 2024

View reviewed changes

...coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs Outdated Show resolved Hide resolved

jkotas reviewed Apr 4, 2024

View reviewed changes

...em.Runtime/tests/System.Runtime.Tests/System/Runtime/CompilerServices/RuntimeHelpersTests.cs Show resolved Hide resolved

Check ContainsGenericVariables

3423a52

jkotas reviewed Apr 5, 2024

View reviewed changes

...veaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.NativeAot.cs Outdated Show resolved Hide resolved

jkotas reviewed Apr 5, 2024

View reviewed changes

...coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs Show resolved Hide resolved

Update RuntimeHelpers.NativeAot.cs

1c473f6

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

jkoritzinsky added 3 commits April 5, 2024 10:14

Add doc comments

21fde4e

Separate out partially-specialized type test

51a93b2

Merge branch 'sizeof' of github.com:jkoritzinsky/runtime into sizeof

ec81d77

jkotas reviewed Apr 5, 2024

View reviewed changes

This was referenced Apr 5, 2024

[browser] Unable to evaluate script: tab crashed #100275

Closed

System.IO.Net5Compat.Tests and System.IO.Tests suddenly exiting with error 137 #100558

Closed

Update doc comment to refer to Unsafe.SizeOf

ed05d1a

build-analysis bot mentioned this pull request Apr 6, 2024

ComInterfaceGenerator.Unit.Tests failed on linux-x64 running interpreter #100683

Closed

EgorBo mentioned this pull request Apr 7, 2024

Align up structs to IntPtr.Size if they have gc pointers #100289

Merged

am11 reviewed Apr 8, 2024

View reviewed changes

This was referenced Apr 8, 2024

[browser][MT] GC deadlock #96190

Closed

MONO interpreter: NIY encountered in method #98730

Closed

jkoritzinsky mentioned this pull request Apr 9, 2024

Mono Wasm V8 System.Text.Encodings.Web.Tests failure #100830

Closed

jkoritzinsky merged commit f54c778 into dotnet:main Apr 9, 2024

jkoritzinsky deleted the sizeof branch April 9, 2024 19:55

matouskozak pushed a commit to matouskozak/runtime that referenced this pull request Apr 30, 2024

Implement RuntimeHelpers.SizeOf (dotnet#100618)

90795f5

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

github-actions bot locked and limited conversation to collaborators May 10, 2024

	/// This API has the same behavior as if you were to use the IL sizeof instruction with the passed in type as the operand.
	/// This API returns the same value as the IL sizeof instruction with the passed in type as the operand.

Conversation

jkoritzinsky commented Apr 3, 2024

Uh oh!

ghost commented Apr 3, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jkotas Apr 5, 2024

Choose a reason for hiding this comment

Uh oh!

jkotas Apr 5, 2024

Choose a reason for hiding this comment

Uh oh!

am11 Apr 8, 2024

Choose a reason for hiding this comment

Uh oh!

jkoritzinsky Apr 8, 2024

Choose a reason for hiding this comment

Uh oh!

jkoritzinsky commented Apr 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants