Skip to content

Fixed extra emitted pop when consuming IL types with init properties#11552

Merged
TIHan merged 9 commits intodotnet:mainfrom
TIHan:init-pop-fix
May 12, 2021
Merged

Fixed extra emitted pop when consuming IL types with init properties#11552
TIHan merged 9 commits intodotnet:mainfrom
TIHan:init-pop-fix

Conversation

@TIHan
Copy link
Copy Markdown
Contributor

@TIHan TIHan commented May 12, 2021

C#:

namespace ClassLibrary1
{
    public record Test
    {
        public int X { get; init; }
    }
}

F#:

open ClassLibrary1
open System.Runtime.CompilerServices

[<MethodImpl(MethodImplOptions.NoInlining)>]
let test() =
    Test(X = 1)

[<EntryPoint>]
let main argv =
    test() |> ignore
    0

The emitted IL from the F# code:

.method public static class [ClassLibrary1]ClassLibrary1.Test 
        test() cil managed noinlining
{
  // Code size       16 (0x10)
  .maxstack  4
  .locals init (class [ClassLibrary1]ClassLibrary1.Test V_0)
  IL_0000:  newobj     instance void [ClassLibrary1]ClassLibrary1.Test::.ctor()
  IL_0005:  stloc.0
  IL_0006:  ldloc.0
  IL_0007:  ldc.i4.1
  IL_0008:  callvirt   instance void modreq([System.Runtime]System.Runtime.CompilerServices.IsExternalInit) [ClassLibrary1]ClassLibrary1.Test::set_X(int32)
  IL_000d:  pop
  IL_000e:  ldloc.0
  IL_000f:  ret
} // end of method Program::test

Notice the IL_000d: pop - there should not be one here as there is nothing to pop and results in invalid IL and causes the runtime to throw.

The PR should fix this by stripping away ILType.Modified IL types when checking for a return type on an IL call.

@TIHan TIHan changed the title Fixed extra emitted pop when consuming IL types with init properties Fixed extra emitted pop when consuming IL types with init properties May 12, 2021
@TIHan TIHan mentioned this pull request May 12, 2021
Closed
5 tasks
baronfel
baronfel reviewed May 12, 2021
View reviewed changes
Comment thread src/fsharp/absil/il.fs Outdated
@TIHan TIHan mentioned this pull request May 12, 2021
Closed
@TIHan TIHan merged commit 9917b38 into dotnet:main May 12, 2021
@TIHan TIHan deleted the init-pop-fix branch May 12, 2021 16:28
@smoothdeveloper smoothdeveloper mentioned this pull request May 14, 2021
Closed
3 tasks
chrissimon-au added a commit to dev-cycles/contextive that referenced this pull request Nov 2, 2021
@chrissimon-au
Update dev container to .net 6 to allow use of c# record with init pr…
564aa4f 
…operties - see dotnet/fsharp#11552; update debug and testing tasks
chrissimon-au added a commit to dev-cycles/contextive that referenced this pull request Nov 3, 2021
@chrissimon-au
Language server config (#2)
52b8c96 
* Update dev container to .net 6 to allow use of c# record with init properties - see dotnet/fsharp#11552; update debug and testing tasks

* Initial cut of server requesting config values on startup

* Switch to mailboxprocessor based waiting for startup log message

* Refactor condition awaiter into generic

* Refactor configurationSection
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@baronfel baronfel baronfel left review comments

@dsyme dsyme dsyme approved these changes

+1 more reviewer

@cartermp cartermp cartermp approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@TIHan @baronfel @cartermp @dsyme