Make TypeBuilder.CreateType return non nullable #72180

JosieBigler · 2022-07-14T13:16:47Z

There is no situation when TypeBuilder.CreateType will return null when
being called by user-code.

There is no situation when TypeBuilder.CreateType will return null when being called by user-code. issue dotnet#68840

ghost · 2022-07-14T13:17:00Z

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

Issue Details

There is no situation when TypeBuilder.CreateType will return null when
being called by user-code.

issue #68840

Author:	JosieBigler
Assignees:	-
Labels:	`area-System.Reflection`
Milestone:	-

dnfadmin · 2022-07-14T13:17:02Z

All CLA requirements met.

src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs

ghost · 2022-07-14T14:30:10Z

Tagging subscribers to this area: @dotnet/area-system-reflection-emit
See info in area-owners.md if you want to be subscribed.

Issue Details

There is no situation when TypeBuilder.CreateType will return null when
being called by user-code.

issue #68840

Author:	JosieBigler
Assignees:	-
Labels:	`area-System.Reflection`, `area-System.Reflection.Emit`, `community-contribution`
Milestone:	-

src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs

src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderCreateType.cs

src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs

stephentoub · 2022-07-22T20:00:09Z

Should EnumBuilder.CreateType be annotated differently as well?

What about CreateTypeInfo?

Can we remove the ! from

runtime/src/libraries/System.Reflection.DispatchProxy/src/System/Reflection/DispatchProxyGenerator.cs

Line 312 in 0c32267

return new GeneratedTypeInfo(_tb.CreateType()!, _methodInfos.ToArray());

?

…/runtime into create-type-not-null

JosieBigler · 2022-07-26T04:32:13Z

@stephentoub I've removed the ! from DispatchProxyGenerator.cs.

It looks like we can change CreateTypeInfo . There are a few more null-forgiving operators associated with CreateTypeInfo mostly in the System.Private.Xml Do you want me to remove the ! from there as well? Or Should these changes stay confined to System.Reflection?

src/libraries/System.Reflection.Emit/tests/ModuleBuilder/ModuleBuilderCreateGlobalFunctions.cs

jkotas · 2022-07-26T05:32:20Z

There are a few more null-forgiving operators associated with CreateTypeInfo mostly in the System.Private.Xml Do you want me to remove the ! from there as well?

Yes, you can remove the no-unnecessary ! from everywhere. Also, you should be able to delete the unnecessary AsType calls: replace CreateTypeInfo()!.AsType() with just CreateType().

src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/EnumBuilder.cs

src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs

Refactored CreateTypeInfo().AsType() to be CreateType()

jkotas · 2022-07-26T15:25:42Z

src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs

+        }
+
+        [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)]
+        internal TypeInfo? CreateTypeInfoImpl()


CreateTypeInfoImpl is identical to CreateTypeImpl. Could you please delete CreateTypeImpl and make CreateTypeInfo to call CreateTypeImpl instead?

Changed to only have 1 internal method with a signature of internal TypeInfo? CreateTypeInfoImpl().

jkotas · 2022-07-26T15:29:03Z

Could you please merge from main to resolve conflict in TypeBuilderAddInterfaceImplementation.cs?

jkotas · 2022-07-26T15:30:06Z

src/libraries/System.Private.Xml/src/System/Xml/Xsl/IlGen/XmlILModule.cs

@@ -239,7 +239,7 @@ public void BakeMethods()

            if (!_useLRE)
            {
-                typBaked = _typeBldr!.CreateTypeInfo()!.AsType();
+                typBaked = _typeBldr.CreateType();


Suggested change

typBaked = _typeBldr.CreateType();

typBaked = _typeBldr!.CreateType();

I think this needs to stay.

Added back the ! to _typeBldr.

…g operator.

src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs

jkotas

LGMT. Thank you!

lonix1 · 2022-07-27T00:52:44Z

@JosieBigler You never thought it would get this complicated when you volunteered... 😄 Seriously though, well done and thanks!

Make TypeBuilder.CreateType return non nullable

aeca88e

There is no situation when TypeBuilder.CreateType will return null when being called by user-code. issue dotnet#68840

dotnet-issue-labeler bot added the area-System.Reflection label Jul 14, 2022

ghost added the community-contribution Indicates that the PR has been added by a community member label Jul 14, 2022

stephentoub reviewed Jul 14, 2022

View reviewed changes

src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs Outdated Show resolved Hide resolved

jkotas added area-System.Reflection.Emit and removed area-System.Reflection labels Jul 14, 2022

Fix unconditional throw and use CreateGlobalModuleType in ModelBuilder

4ea1115

buyaa-n reviewed Jul 14, 2022

View reviewed changes

src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs Outdated Show resolved Hide resolved

Fix code per Review Comments

cefc51d

This was referenced Jul 19, 2022

system.net.quic.functional.tests failing with stack buffer overflow #72429

Closed

System.Net.Http.Functional.Tests.SyncHttpHandler_DiagnosticsTest.SendAsync_SuppressedGlobalStaticPropagationEnvVar failing in CI #72428

Closed

buyaa-n reviewed Jul 20, 2022

View reviewed changes

src/libraries/System.Reflection.Emit/tests/TypeBuilder/TypeBuilderCreateType.cs Outdated Show resolved Hide resolved

buyaa-n reviewed Jul 20, 2022

View reviewed changes

src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs Outdated Show resolved Hide resolved

JosieBigler added 2 commits July 21, 2022 20:03

Update tests for null check after module.CreateGlobalFunctions()

a52411b

Check not null for additional CreateType Unit Tests

af2d6ee

JosieBigler added 5 commits July 22, 2022 20:33

Remove ! from DispatchProxyGenerator CreateType()

05f3faa

Pulled in changes from main fork.

036d988

Recommit changes after pulling main fork

4914a41

Merge branch 'create-type-not-null' of https://github.com/JosieBigler…

bc20792

…/runtime into create-type-not-null

Update Ref for non-null CreateType

c0548e9

buyaa-n reviewed Jul 26, 2022

View reviewed changes

src/libraries/System.Reflection.Emit/tests/ModuleBuilder/ModuleBuilderCreateGlobalFunctions.cs Outdated Show resolved Hide resolved

jkotas reviewed Jul 26, 2022

View reviewed changes

src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/EnumBuilder.cs Show resolved Hide resolved

jkotas reviewed Jul 26, 2022

View reviewed changes

src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs Show resolved Hide resolved

JosieBigler mentioned this pull request Jul 26, 2022

Assert.Null failed for mono in System.Reflection.Emit.Tests #72858

Closed

JosieBigler added 4 commits July 26, 2022 09:47

Change CreateTypeInfo to return non-null.

c37e63c

Refactored CreateTypeInfo().AsType() to be CreateType()

Remove null-forgiving operator

f695771

Annotate failing Mono Assert.Null

c36a427

Merge remote-tracking branch 'upstream/main' into create-type-not-null

1f19115

jkotas reviewed Jul 26, 2022

View reviewed changes

JosieBigler added 2 commits July 26, 2022 11:14

Only 1 internal method that returns TypeInfo?. Add back null-forgivin…

7f3ec14

…g operator.

Only skip mono tests for null check Asserts, not the entire test.

f15cb78

jkotas reviewed Jul 26, 2022

View reviewed changes

src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.cs Show resolved Hide resolved

jkotas approved these changes Jul 26, 2022

View reviewed changes

jkotas merged commit da46270 into dotnet:main Jul 26, 2022

ghost locked as resolved and limited conversation to collaborators Aug 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make TypeBuilder.CreateType return non nullable #72180

Make TypeBuilder.CreateType return non nullable #72180

JosieBigler commented Jul 14, 2022 •

edited by buyaa-n

Loading

ghost commented Jul 14, 2022

dnfadmin commented Jul 14, 2022 •

edited

Loading

ghost commented Jul 14, 2022

stephentoub commented Jul 22, 2022

JosieBigler commented Jul 26, 2022

jkotas commented Jul 26, 2022

jkotas Jul 26, 2022

JosieBigler Jul 26, 2022

jkotas commented Jul 26, 2022

jkotas Jul 26, 2022

JosieBigler Jul 26, 2022

jkotas left a comment

lonix1 commented Jul 27, 2022

	typBaked = _typeBldr.CreateType();
	typBaked = _typeBldr!.CreateType();

Make TypeBuilder.CreateType return non nullable #72180

Make TypeBuilder.CreateType return non nullable #72180

Conversation

JosieBigler commented Jul 14, 2022 • edited by buyaa-n Loading

ghost commented Jul 14, 2022

dnfadmin commented Jul 14, 2022 • edited Loading

ghost commented Jul 14, 2022

stephentoub commented Jul 22, 2022

JosieBigler commented Jul 26, 2022

jkotas commented Jul 26, 2022

jkotas Jul 26, 2022

Choose a reason for hiding this comment

JosieBigler Jul 26, 2022

Choose a reason for hiding this comment

jkotas commented Jul 26, 2022

jkotas Jul 26, 2022

Choose a reason for hiding this comment

JosieBigler Jul 26, 2022

Choose a reason for hiding this comment

jkotas left a comment

Choose a reason for hiding this comment

lonix1 commented Jul 27, 2022

JosieBigler commented Jul 14, 2022 •

edited by buyaa-n

Loading

dnfadmin commented Jul 14, 2022 •

edited

Loading