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

ILASM/ILDASM support for covariant returns #37045

Closed
MichalStrehovsky opened this issue May 27, 2020 · 3 comments
Closed

ILASM/ILDASM support for covariant returns #37045

MichalStrehovsky opened this issue May 27, 2020 · 3 comments
Assignees
Milestone

Comments

@MichalStrehovsky
Copy link
Member

The shortcut .override Class::Method ILASM syntax is generating unresolvable MemberRefs for covariant returns (only the more wordy .override method instance ReturnType Class::Method(Param1, Param2) syntax works).

ILDASM generates the shortcut syntax, things don't roundtrip. I found this when I was playing with this in the linker yesterday.

To repro, e.g. take the CompatibleWith.il test, ILASM it, then ILDASM it and ILASM it again.

After the first ILDASM:

.method /*06000009*/ public hidebysig newslot virtual 
        instance class [System.Runtime/*23000003*/]System.Collections.Generic.IList`1/*01000002*/<int32> 
        M1() cil managed
{
  .override C1/*02000004*/::M1 /*02000004::06000002*/ 
  // Code size       1 (0x1)
  .maxstack  8
  IL_0000:  ret
} // end of method C2::M1

After the second ILDASM:

.method /*06000009*/ public hidebysig newslot virtual 
        instance class [System.Runtime/*23000003*/]System.Collections.Generic.IList`1/*01000002*/<int32> 
        M1() cil managed
{
  .override C1/*01000006*/::M1 /*01000006::0A00000D*/ 
  // Code size       1 (0x1)
  .maxstack  8
  IL_0000:  ret
} // end of method C2::M1

Notice that the .override is actually using MemberRefs in the second disassembly and I don't think the MemberRefs actually resolve (didn't try with Fadi's/Jan's branch, but I don't think it will work).

The MemberRef token is defined as this:

TypeRef #6 (01000006)
-------------------------------------------------------
Token:             0x01000006
ResolutionScope:   0x00000001
TypeRefName:       C1
	MemberRef #1 (0a00000d)
	-------------------------------------------------------
		Member: (0a00000d) M1: 
		CallCnvntn: [DEFAULT]
		hasThis 
		ReturnType: GenericInst Class System.Collections.Generic.IList`1< I4>
		No arguments.

Notice the return type.

It's also weird there's no warning about this from ILASM, but that's a preexisting issue I guess.

@MichalStrehovsky MichalStrehovsky added this to the 5.0 milestone May 27, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label May 27, 2020
@MichalStrehovsky
Copy link
Member Author

Cc @janvorli

@BruceForstall BruceForstall removed the untriaged New issue has not been triaged by the area owner label Jun 1, 2020
@BruceForstall
Copy link
Member

@MichalStrehovsky Someone working on the covariant return feature needs to own this implementation (and take ownership of this bug). Who would that be?

@MichalStrehovsky
Copy link
Member Author

@janvorli who is already Cc'd above

@janvorli janvorli self-assigned this Jun 2, 2020
@davidwrighton davidwrighton self-assigned this Aug 7, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants