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

In order to reflect nullability in constraint types, compiler emits Nullable attribute for rows in GenericParamConstraint metadata table #29997

Closed
3 of 4 tasks
AlekseyTs opened this issue Sep 18, 2018 · 15 comments

Comments

@AlekseyTs
Copy link
Contributor

AlekseyTs commented Sep 18, 2018

It looks like there is no syntax to represent this in IL. Is this a problem?

Example: class C<T> where T : Interface?

Relates to dotnet/linker#626

@AlekseyTs AlekseyTs changed the title In order to reflect nullability in constraint types compiler emits Nullable attribute for rows in GenericParamConstraint metadata table In order to reflect nullability in constraint types, compiler emits Nullable attribute for rows in GenericParamConstraint metadata table Sep 18, 2018
@jcouv jcouv added this to the 16.0 milestone Sep 20, 2018
@jaredpar jaredpar added this to Misc in Nullable Board Jan 28, 2019
@jcouv jcouv moved this from Misc to Needs Design in Nullable Board Jan 31, 2019
@gafter gafter modified the milestones: 16.0, 16.1 Feb 26, 2019
@jcouv jcouv modified the milestones: 16.1, 16.2 Apr 23, 2019
@jcouv jcouv modified the milestones: 16.2, 16.3 Jun 18, 2019
@jcouv
Copy link
Member

jcouv commented Jun 18, 2019

Tagging @safern @stephentoub @agocke

The issue is that we're using metadata representation which is legal but not representable in IL.
We did this in the past for tuple names, but we're doing this more for nullability in constraints and nullability in method interface implementations (thx Aleksey for correction).

At this point in the release cycle, I think we will stick with this design unless there is a major problem and we have a viable alternative. I'll keep this issue open for another week or two to give you the chance to chime in, then I'll close.

@stephentoub
Copy link
Member

cc: @jkotas

@jkotas
Copy link
Member

jkotas commented Jun 19, 2019

Does this mean that you won't be able to round-trip these assemblies using ildasm/ilasm? Number of folks out there who depend on ilasm/ildasm round-tripping as part of their build...

@jcouv
Copy link
Member

jcouv commented Jun 19, 2019

That's correct. We would need to update ildasm/ilasm to allow such round-tripping.

@AlekseyTs Could you add details on the method implementation scenario (where is the attribute placed?). Thanks

@jkotas
Copy link
Member

jkotas commented Jun 19, 2019

Also, what about Cecil and all tools build on top of it? (e.g. https://github.com/mono/linker that is officially supported tool for .NET Core 3.0)

@AlekseyTs
Copy link
Contributor Author

@jcouv

Could you add details on the method implementation scenario (where is the attribute placed?).

I was talking about interface implementations. I do not think we emit any attributes for MethodIlmpls.

@AlekseyTs
Copy link
Contributor Author

As simple as class C : MyInterface<string?>

@jbevain
Copy link
Contributor

jbevain commented Jun 20, 2019

@jcouv @jkotas That would be a breaking change in Cecil albeit a similar one we did to InterfaceImplementation in 0.10.

That would need to go in 0.11 and should be an easy change in Cecil upstream, the linker would need to be updated to adopt the change.

@jcouv
Copy link
Member

jcouv commented Jun 21, 2019

It looks like the interface implementation scenario is supported by IL/ilasm/ildasm after all. See details below.
This means we only have to worry about the type constraint scenario (such as where T : Interface?).

Interface implementation appears to be supported:

From the following C#:

#nullable enable
public interface I<T> { }
public interface I2<T> { }
public class D<T, U> : I<T?>, I2<U> where T : class where U : class
{ 
}

ildasm produces the following IL:

.class public auto ansi beforefieldinit D`2<class T,class U>
       extends [mscorlib]System.Object
       implements class I`1<!T>,
                  class I2`1<!U>
{
  .interfaceimpl type class I`1<!T>
  .custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8[]) = ( 01 00 02 00 00 00 00 02 00 00 ) 
  .interfaceimpl type class I2`1<!U>
  .custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8[]) = ( 01 00 02 00 00 00 00 01 00 00 ) 
  .param type T 
  .custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 01 00 00 ) 
  .param type U 
  .custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 01 00 00 ) 
} // end of class D`2

And ildasm produces the following MetaInfo:


TypeDef #6 (02000007)
-------------------------------------------------------
	TypDefName: D`2  (02000007)
	Flags     : [Public] [AutoLayout] [Class] [AnsiClass] [BeforeFieldInit]  (00100001)
	Extends   : 01000007 [TypeRef] System.Object
	2 Generic Parameters
		(0) GenericParamToken : (2a000004) Name : T flags: 00000004 Owner: 02000007
		CustomAttribute #1 (0c00000d)
		-------------------------------------------------------
			CustomAttribute Type: 06000002
			CustomAttributeName: System.Runtime.CompilerServices.NullableAttribute :: instance void .ctor(unsigned int8)
			Length: 5
			Value : 01 00 01 00 00                                   >                <
			ctor args: (1)

		(1) GenericParamToken : (2a000005) Name : U flags: 00000004 Owner: 02000007
		CustomAttribute #1 (0c00000e)
		-------------------------------------------------------
			CustomAttribute Type: 06000002
			CustomAttributeName: System.Runtime.CompilerServices.NullableAttribute :: instance void .ctor(unsigned int8)
			Length: 5
			Value : 01 00 01 00 00                                   >                <
			ctor args: (1)

	Method #1 (06000006) 
	-------------------------------------------------------
		MethodName: .ctor (06000006)
		Flags     : [Public] [HideBySig] [ReuseSlot] [SpecialName] [RTSpecialName] [.ctor]  (00001886)
		RVA       : 0x00002085
		ImplFlags : [IL] [Managed]  (00000000)
		CallCnvntn: [DEFAULT]
		hasThis 
		ReturnType: Void
		No arguments.

	InterfaceImpl #1 (09000001)
	-------------------------------------------------------
		Class     : D`2
		Token     : 1B000001 [TypeSpec] [TypeSpec]
		CustomAttribute #1 (0c000001)
		-------------------------------------------------------
			CustomAttribute Type: 06000003
			CustomAttributeName: System.Runtime.CompilerServices.NullableAttribute :: instance void .ctor(unsigned int8[])
			Length: 10
			Value : 01 00 02 00 00 00 00 02  00 00                   >                <
			ctor args: ( <can not decode> )


	InterfaceImpl #2 (09000002)
	-------------------------------------------------------
		Class     : D`2
		Token     : 1B000002 [TypeSpec] [TypeSpec]
		CustomAttribute #1 (0c000009)
		-------------------------------------------------------
			CustomAttribute Type: 06000003
			CustomAttributeName: System.Runtime.CompilerServices.NullableAttribute :: instance void .ctor(unsigned int8[])
			Length: 10
			Value : 01 00 02 00 00 00 00 01  00 00                   >                <
			ctor args: ( <can not decode> )

@sbomer
Copy link
Member

sbomer commented Jun 21, 2019

Are the attributes on generic param constraints going to be emitted in 3.0?

@jcouv
Copy link
Member

jcouv commented Jun 21, 2019

Are the attributes on generic param constraints going to be emitted in 3.0?

Yes. They will be emitted by anyone using C# 8.0 (nullability analysis feature), which ships with Core 3.0.

@jbevain
Copy link
Contributor

jbevain commented Jun 24, 2019

@jcouv is there a scenario where in the metadata you could generate a CustomAttr owned by a TypeRef or a TypeSpec?

@AlekseyTs
Copy link
Contributor Author

is there a scenario where in the metadata you could generate a CustomAttr owned by a TypeRef or a TypeSpec?

In general, yes. See AssemblyAttributesGoHere type and the like, for example? However, compiler never attaches NullableAttribute to a TypeRef.

@jbevain
Copy link
Contributor

jbevain commented Jun 25, 2019

@AlekseyTs I know the metadata format allows it. Just wanted to make sure that adding support for custom attributes on generic parameter constraints is enough to support this feature we're discussing.

@jaredpar
Copy link
Member

jaredpar commented Feb 9, 2021

Closing as the runtime team added support for this in the ilasm / ildasm toolset. Can re-open if it's not sufficient.

@jaredpar jaredpar closed this as completed Feb 9, 2021
amanda-tarafa added a commit to amanda-tarafa/google-cloud-dotnet that referenced this issue Aug 3, 2021
For nullability support.

Mono.Cecil introduced a breaking change in jbevain/cecil#594 because of dotnet/roslyn#29997
jskeet pushed a commit to googleapis/google-cloud-dotnet that referenced this issue Aug 4, 2021
For nullability support.

Mono.Cecil introduced a breaking change in jbevain/cecil#594 because of dotnet/roslyn#29997
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment