Skip to content

Commit 3a393bd

Browse files
T-GroCopilot
andcommitted
Fix method signatures and calling conventions
Fix 4 codegen bugs in method call emission and parameter metadata: - #13468: outref parameter compiled as byref - #18135: Static abstract interface members with byref params - #18140: callvirt on value types causes ILVerify errors - #18374: RuntimeWrappedException cannot be caught Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent ee6bc91 commit 3a393bd

17 files changed

Lines changed: 293 additions & 67 deletions

File tree

docs/release-notes/.FSharp.Compiler.Service/10.0.300.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414
* Fix FS0229 B-stream misalignment when reading metadata from assemblies compiled with LangVersion < 9.0, introduced by [#17706](https://github.com/dotnet/fsharp/pull/17706). ([PR #19260](https://github.com/dotnet/fsharp/pull/19260))
1515
* Fix FS3356 false positive for instance extension members with same name on different types, introduced by [#18821](https://github.com/dotnet/fsharp/pull/18821). ([PR #19260](https://github.com/dotnet/fsharp/pull/19260))
1616
* F# Scripts: Fix default reference paths resolving when an SDK directory is specified. ([PR #19270](https://github.com/dotnet/fsharp/pull/19270))
17+
* Fix outref parameter compiled as byref. (Issue [#13468](https://github.com/dotnet/fsharp/issues/13468), [PR #19340](https://github.com/dotnet/fsharp/pull/19340))
18+
* Fix static abstract interface members with byref params. (Issue [#18135](https://github.com/dotnet/fsharp/issues/18135), [PR #19340](https://github.com/dotnet/fsharp/pull/19340))
19+
* Use constrained.callvirt for method calls on value types. (Issue [#18140](https://github.com/dotnet/fsharp/issues/18140), [PR #19340](https://github.com/dotnet/fsharp/pull/19340))
1720

1821
### Added
1922

src/Compiler/AbstractIL/ilwrite.fs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -446,16 +446,24 @@ type MethodDefKey(ilg:ILGlobals, tidx: int, garity: int, nm: string, retTy: ILTy
446446
override _.Equals(obj: obj) =
447447
match obj with
448448
| :? MethodDefKey as y ->
449-
let compareILTypes o1 o2 =
449+
let rec compareILTypes o1 o2 =
450450
match o1, o2 with
451-
| ILType.Value v1, ILType.Value v2 -> v1.EqualsWithPrimaryScopeRef(ilg.primaryAssemblyScopeRef, v2 :> obj )
451+
| ILType.Value v1, ILType.Value v2 -> v1.EqualsWithPrimaryScopeRef(ilg.primaryAssemblyScopeRef, v2 :> obj)
452+
| ILType.Boxed v1, ILType.Boxed v2 -> v1.EqualsWithPrimaryScopeRef(ilg.primaryAssemblyScopeRef, v2 :> obj)
453+
| ILType.Byref t1, ILType.Byref t2 -> compareILTypes t1 t2
454+
| ILType.Ptr t1, ILType.Ptr t2 -> compareILTypes t1 t2
455+
| ILType.Array(sh1, t1), ILType.Array(sh2, t2) -> sh1 = sh2 && compareILTypes t1 t2
456+
| ILType.Modified(req1, tref1, t1), ILType.Modified(req2, tref2, t2) ->
457+
req1 = req2 && tref1.EqualsWithPrimaryScopeRef(ilg.primaryAssemblyScopeRef, tref2 :> obj) && compareILTypes t1 t2
458+
| ILType.Modified(_, _, t1), t2 -> compareILTypes t1 t2
459+
| t1, ILType.Modified(_, _, t2) -> compareILTypes t1 t2
452460
| _ -> o1 = o2
453461

454462
tidx = y.TypeIdx &&
455463
garity = y.GenericArity &&
456464
nm = y.Name &&
457-
// note: these next two use structural equality on AbstractIL ILType values
458-
retTy = y.ReturnType && List.lengthsEqAndForall2 compareILTypes argTys y.ArgTypes &&
465+
// note: these next two use scope-aware equality on AbstractIL ILType values
466+
compareILTypes retTy y.ReturnType && List.lengthsEqAndForall2 compareILTypes argTys y.ArgTypes &&
459467
isStatic = y.IsStatic
460468
| _ -> false
461469

src/Compiler/CodeGen/IlxGen.fs

Lines changed: 46 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -605,6 +605,10 @@ let voidCheck m g permits ty =
605605
[<Struct>]
606606
type DuFieldCoordinates = { CaseIdx: int; FieldIdx: int }
607607

608+
/// Flags propagated from interface slot signatures to parameter metadata.
609+
[<Struct>]
610+
type SlotParamFlags = { IsIn: bool; IsOut: bool }
611+
608612
/// Structure for maintaining field reuse across struct unions
609613
type UnionFieldReuseMap = MultiMap<string, DuFieldCoordinates>
610614

@@ -3491,7 +3495,6 @@ and GenLinearExpr cenv cgbuf eenv expr sequel preSteps (contf: FakeUnit -> FakeU
34913495
if preSteps && GenExprPreSteps cenv cgbuf eenv expr sequel then
34923496
contf Fake
34933497
else
3494-
34953498
// This case implemented here to get a guaranteed tailcall
34963499
// Make sure we generate the debug point outside the scope of the variable
34973500
let startMark, endMark as scopeMarks = StartDelayedLocalScope "let" cgbuf
@@ -5423,6 +5426,7 @@ and GenILCall
54235426
(virt, valu, newobj, valUseFlags, isDllImport, ilMethRef: ILMethodRef, enclArgTys, methArgTys, argExprs, returnTys, m)
54245427
sequel
54255428
=
5429+
let g = cenv.g
54265430
let hasByrefArg = ilMethRef.ArgTypes |> List.exists IsILTypeByref
54275431

54285432
let isSuperInit =
@@ -5445,6 +5449,24 @@ and GenILCall
54455449
let makesNoCriticalTailcalls = (newobj || not virt) // Don't tailcall for 'newobj', or 'call' to IL code
54465450
let hasStructObjArg = valu && ilMethRef.CallingConv.IsInstance
54475451

5452+
let ilEnclArgTys = GenTypeArgs cenv m eenv.tyenv enclArgTys
5453+
let ilMethArgTys = GenTypeArgs cenv m eenv.tyenv methArgTys
5454+
let ilReturnTys = GenTypes cenv m eenv.tyenv returnTys
5455+
let ilMethSpec = mkILMethSpec (ilMethRef, boxity, ilEnclArgTys, ilMethArgTys)
5456+
5457+
let useICallVirt =
5458+
(virt || useCallVirt cenv boxity ilMethSpec isBaseCall)
5459+
&& ilMethRef.CallingConv.IsInstance
5460+
5461+
let ccallInfo =
5462+
ccallInfo
5463+
|> Option.orElseWith (fun () ->
5464+
match argExprs with
5465+
| objArgExpr :: _ when useICallVirt ->
5466+
let objArgTy = tyOfExpr g objArgExpr
5467+
if isStructTy g objArgTy then Some objArgTy else None
5468+
| _ -> None)
5469+
54485470
let tail =
54495471
CanTailcall(
54505472
hasStructObjArg,
@@ -5459,15 +5481,6 @@ and GenILCall
54595481
sequel
54605482
)
54615483

5462-
let ilEnclArgTys = GenTypeArgs cenv m eenv.tyenv enclArgTys
5463-
let ilMethArgTys = GenTypeArgs cenv m eenv.tyenv methArgTys
5464-
let ilReturnTys = GenTypes cenv m eenv.tyenv returnTys
5465-
let ilMethSpec = mkILMethSpec (ilMethRef, boxity, ilEnclArgTys, ilMethArgTys)
5466-
5467-
let useICallVirt =
5468-
(virt || useCallVirt cenv boxity ilMethSpec isBaseCall)
5469-
&& ilMethRef.CallingConv.IsInstance
5470-
54715484
// Load the 'this' pointer to pass to the superclass constructor. This argument is not
54725485
// in the expression tree since it can't be treated like an ordinary value
54735486
if isSuperInit then
@@ -8975,6 +8988,7 @@ and GenParams
89758988
(argInfos: ArgReprInfo list)
89768989
methArgTys
89778990
(implValsOpt: Val list option)
8991+
(slotSigParamFlags: SlotParamFlags list option)
89788992
=
89798993
let g = cenv.g
89808994
let ilWitnessParams = GenWitnessParams cenv eenv m witnessInfos
@@ -8993,11 +9007,18 @@ and GenParams
89939007
| _ -> List.map (fun x -> x, None) ilArgTysAndInfos
89949008

89959009
let ilParams, _ =
8996-
(Set.empty, List.zip methArgTys ilArgTysAndInfoAndVals)
8997-
||> List.mapFold (fun takenNames (methodArgTy, ((ilArgTy, topArgInfo), implValOpt)) ->
9010+
((Set.empty, 0), List.zip methArgTys ilArgTysAndInfoAndVals)
9011+
||> List.mapFold (fun (takenNames, paramIdx) (methodArgTy, ((ilArgTy, topArgInfo), implValOpt)) ->
89989012
let inFlag, outFlag, optionalFlag, defaultParamValue, Marshal, attribs =
89999013
GenParamAttribs cenv methodArgTy topArgInfo.Attribs
90009014

9015+
let inFlag, outFlag =
9016+
match slotSigParamFlags with
9017+
| Some flags when paramIdx < flags.Length ->
9018+
let slotFlags = flags[paramIdx]
9019+
(inFlag || slotFlags.IsIn, outFlag || slotFlags.IsOut)
9020+
| _ -> (inFlag, outFlag)
9021+
90019022
let idOpt =
90029023
match topArgInfo.Name with
90039024
| Some v -> Some v
@@ -9036,7 +9057,7 @@ and GenParams
90369057
MetadataIndex = NoMetadataIdx
90379058
}
90389059

9039-
param, takenNames)
9060+
param, (takenNames, paramIdx + 1))
90409061

90419062
ilWitnessParams @ ilParams
90429063

@@ -9389,8 +9410,18 @@ and GenMethodForBinding
93899410

93909411
let ilTypars = GenGenericParams cenv eenvUnderMethLambdaTypars methLambdaTypars
93919412

9413+
let slotSigParamFlags =
9414+
match v.ImplementedSlotSigs with
9415+
| slotsig :: _ ->
9416+
let slotParams = slotsig.FormalParams |> List.concat
9417+
9418+
slotParams
9419+
|> List.map (fun (TSlotParam(_, _, inFlag, outFlag, _, _)) -> { IsIn = inFlag; IsOut = outFlag })
9420+
|> Some
9421+
| [] -> None
9422+
93929423
let ilParams =
9393-
GenParams cenv eenvUnderMethTypeTypars m mspec witnessInfos paramInfos argTys (Some nonUnitNonSelfMethodVars)
9424+
GenParams cenv eenvUnderMethTypeTypars m mspec witnessInfos paramInfos argTys (Some nonUnitNonSelfMethodVars) slotSigParamFlags
93949425

93959426
let ilReturn =
93969427
GenReturnInfo cenv eenvUnderMethTypeTypars (Some returnTy) mspec.FormalReturnType retInfo
@@ -10720,7 +10751,7 @@ and GenAbstractBinding cenv eenv tref (vref: ValRef) =
1072010751
let ilReturn =
1072110752
GenReturnInfo cenv eenvForMeth returnTy mspec.FormalReturnType retInfo
1072210753

10723-
let ilParams = GenParams cenv eenvForMeth m mspec [] argInfos methArgTys None
10754+
let ilParams = GenParams cenv eenvForMeth m mspec [] argInfos methArgTys None None
1072410755

1072510756
let compileAsInstance = ValRefIsCompiledAsInstanceMember g vref
1072610757

src/Compiler/Service/FSharpCheckerResults.fs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ open System.Diagnostics
1111
open System.IO
1212
open System.Threading
1313
open FSharp.Compiler.IO
14+
open FSharp.Compiler.Import.Nullness
1415
open FSharp.Compiler.NicePrint
16+
open FSharp.Compiler.TypeHierarchy
1517
open Internal.Utilities.Library
1618
open Internal.Utilities.Library.Extras
1719
open Internal.Utilities.TypeHashing
@@ -2108,6 +2110,16 @@ type internal TypeCheckInfo
21082110
|> Seq.tryFindBack (fun (_, _, _, m) -> equals m range)
21092111
|> Option.map (fun (_, q, _, _) -> FSharpDisplayContext(fun _ -> q.DisplayEnv))
21102112

2113+
member scope.ImportILType(ty: ILType) : FSharpType =
2114+
let amap = tcImports.GetImportMap()
2115+
let assemblyRef = ILAssemblyRef.Create("", None, None, false, None, None)
2116+
let scopeRef = ILScopeRef.Assembly assemblyRef
2117+
2118+
let typ =
2119+
ImportILTypeFromMetadata amap range0 scopeRef [] [] NullableAttributesSource.Empty ty
2120+
2121+
FSharpType(cenv, typ)
2122+
21112123
/// Get the auto-complete items at a location
21122124
member _.GetDeclarations(parseResultsOpt, line, lineStr, partialName, completionContextAtPos, getAllEntities, options) =
21132125
let isSigFile = SourceFileImpl.IsSignatureFile mainInputFileName
@@ -3522,6 +3534,9 @@ type FSharpCheckFileResults
35223534
| None -> None
35233535
| Some(scope, _) -> scope.TryGetCapturedDisplayContext(range)
35243536

3537+
member _.ImportILType(ty: ILType) =
3538+
scopeOptX |> Option.map _.ImportILType(ty)
3539+
35253540
member _.GetAllUsesOfAllSymbolsInFile(?cancellationToken: CancellationToken) =
35263541
match details with
35273542
| None -> Seq.empty

src/Compiler/Service/FSharpCheckerResults.fsi

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,9 @@ type public FSharpCheckFileResults =
274274
member TryGetCapturedType: range -> FSharpType option
275275
member TryGetCapturedDisplayContext: range -> FSharpDisplayContext option
276276

277+
/// Imports a compiled type for subsequent Symbols API use
278+
member ImportILType: ILType -> FSharpType option
279+
277280
/// <summary>Get the items for a declaration list</summary>
278281
///
279282
/// <param name="parsedFileResults">

src/Compiler/Symbols/Symbols.fs

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -442,25 +442,22 @@ type FSharpEntity(cenv: SymbolEnv, entity: EntityRef, tyargs: TType list) =
442442
| Some _ -> None
443443

444444
member x.CompiledRepresentation =
445-
checkIsResolved()
446-
447-
let fail () =
448-
invalidOp $"the type '{x.LogicalName}' does not have a qualified name"
445+
if isUnresolved () then None else
449446

450447
#if !NO_TYPEPROVIDERS
451-
if entity.IsTypeAbbrev || entity.IsProvidedErasedTycon || entity.IsNamespace then fail ()
448+
if entity.IsTypeAbbrev || entity.IsProvidedErasedTycon || entity.IsNamespace then None else
452449
#else
453-
if entity.IsTypeAbbrev || entity.IsNamespace then fail ()
450+
if entity.IsTypeAbbrev || entity.IsNamespace then None else
454451
#endif
455452
match entity.CompiledRepresentation with
456-
| CompiledTypeRepr.ILAsmNamed(tref, _, _) -> tref
457-
| CompiledTypeRepr.ILAsmOpen _ -> fail ()
453+
| CompiledTypeRepr.ILAsmNamed(tref, _, _) -> Some tref
454+
| CompiledTypeRepr.ILAsmOpen _ -> None
458455

459456
member x.QualifiedName =
460-
x.CompiledRepresentation.QualifiedName
457+
x.CompiledRepresentation |> Option.map _.QualifiedName
461458

462459
member x.BasicQualifiedName =
463-
x.CompiledRepresentation.BasicQualifiedName
460+
x.CompiledRepresentation |> Option.map _.BasicQualifiedName
464461

465462
member x.FullName =
466463
checkIsResolved()
@@ -2683,16 +2680,13 @@ type FSharpType(cenv, ty:TType) =
26832680
FSharpType(cenv, stripTyEqnsWrtErasure EraseAll cenv.g ty)
26842681

26852682
member x.BasicQualifiedName =
2686-
let fail () =
2687-
invalidOp $"the type '{x}' does not have a qualified name"
2688-
26892683
protect <| fun () ->
26902684
match stripTyparEqns ty with
26912685
| TType_app(tcref, _, _) ->
26922686
match tcref.CompiledRepresentation with
2693-
| CompiledTypeRepr.ILAsmNamed(tref, _, _) -> tref.BasicQualifiedName
2694-
| CompiledTypeRepr.ILAsmOpen _ -> fail ()
2695-
| _ -> fail ()
2687+
| CompiledTypeRepr.ILAsmNamed(tref, _, _) -> Some tref.BasicQualifiedName
2688+
| CompiledTypeRepr.ILAsmOpen _ -> None
2689+
| _ -> None
26962690

26972691
member _.Instantiate(instantiation:(FSharpGenericParameter * FSharpType) list) =
26982692
let resTy = instType (instantiation |> List.map (fun (tyv, ty) -> tyv.TypeParameter, ty.Type)) ty

src/Compiler/Symbols/Symbols.fsi

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,10 +224,10 @@ type FSharpEntity =
224224
member Namespace: string option
225225

226226
/// Get the fully qualified name of the type or module
227-
member QualifiedName: string
227+
member QualifiedName: string option
228228

229229
/// The fully qualified name of the type or module without strong assembly name.
230-
member BasicQualifiedName: string
230+
member BasicQualifiedName: string option
231231

232232
/// Get the full name of the type or module
233233
member FullName: string
@@ -1165,7 +1165,7 @@ type FSharpType =
11651165
member ErasedType: FSharpType
11661166

11671167
/// The fully qualified name of the type or module without strong assembly name.
1168-
member BasicQualifiedName: string
1168+
member BasicQualifiedName: string option
11691169

11701170
/// Adjust the type by removing any occurrences of type inference variables, replacing them
11711171
/// systematically with lower-case type inference variables such as <c>'a</c>.

src/Compiler/Utilities/Activity.fs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ module internal Activity =
135135

136136
depth this 0
137137

138-
let private activitySource = new ActivitySource(ActivityNames.FscSourceName)
138+
let private activitySource = new ActivitySource(ActivityNames.FscSourceName, "")
139139

140140
let start (name: string) (tags: (string * string) seq) : System.IDisposable | null =
141141
let activity = activitySource.CreateActivity(name, ActivityKind.Internal)
@@ -173,7 +173,8 @@ module internal Activity =
173173

174174
let profilingTags = [| workingSetMB; gc0; gc1; gc2; handles; threads |]
175175

176-
let private profiledSource = new ActivitySource(ActivityNames.ProfiledSourceName)
176+
let private profiledSource =
177+
new ActivitySource(ActivityNames.ProfiledSourceName, "")
177178

178179
let startAndMeasureEnvironmentStats (name: string) : System.IDisposable | null = profiledSource.StartActivity(name)
179180

src/FSharp.Core/math/z.fsi

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,31 +17,31 @@ namespace Microsoft.FSharp.Core
1717
/// <category>Basic Types</category>
1818
type bigint = System.Numerics.BigInteger
1919

20-
/// <summary>Provides a default implementations of F# numeric literal syntax for literals of the form 'dddI' </summary>
20+
/// <summary>Provides a default implementation of F# numeric literal syntax for literals of the form 'dddI' </summary>
2121
///
2222
/// <category>Language Primitives</category>
2323
[<AutoOpen>]
2424
module NumericLiterals =
2525

26-
/// Provides a default implementations of F# numeric literal syntax for literals of the form 'dddI'
26+
/// Provides a default implementation of F# numeric literal syntax for literals of the form 'dddI'
2727
module NumericLiteralI =
28-
/// Provides a default implementations of F# numeric literal syntax for literals of the form 'dddI'
28+
/// Provides a default implementation of F# numeric literal syntax for literals of the form 'dddI'
2929
val FromZero: value: unit -> 'T
3030

31-
/// Provides a default implementations of F# numeric literal syntax for literals of the form 'dddI'
31+
/// Provides a default implementation of F# numeric literal syntax for literals of the form 'dddI'
3232
val FromOne: value: unit -> 'T
3333

34-
/// Provides a default implementations of F# numeric literal syntax for literals of the form 'dddI'
34+
/// Provides a default implementation of F# numeric literal syntax for literals of the form 'dddI'
3535
val FromInt32: value: int32 -> 'T
3636

37-
/// Provides a default implementations of F# numeric literal syntax for literals of the form 'dddI'
37+
/// Provides a default implementation of F# numeric literal syntax for literals of the form 'dddI'
3838
val FromInt64: value: int64 -> 'T
3939

40-
/// Provides a default implementations of F# numeric literal syntax for literals of the form 'dddI'
40+
/// Provides a default implementation of F# numeric literal syntax for literals of the form 'dddI'
4141
val FromString: text: string -> 'T
4242

43-
/// Provides a default implementations of F# numeric literal syntax for literals of the form 'dddI'
43+
/// Provides a default implementation of F# numeric literal syntax for literals of the form 'dddI'
4444
val FromInt64Dynamic: value: int64 -> objnull
4545

46-
/// Provides a default implementations of F# numeric literal syntax for literals of the form 'dddI'
46+
/// Provides a default implementation of F# numeric literal syntax for literals of the form 'dddI'
4747
val FromStringDynamic: text: string -> objnull

src/FSharp.Core/prim-types.fs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5995,8 +5995,8 @@ namespace Microsoft.FSharp.Core
59955995
}
59965996

59975997
let current () =
5998-
// according to IEnumerator<int>.Current documentation, the result of of Current
5999-
// is undefined prior to the first call of MoveNext and post called to MoveNext
5998+
// according to IEnumerator<int>.Current documentation, the result of Current
5999+
// is undefined prior to the first call of MoveNext and post call to MoveNext
60006000
// that return false (see https://learn.microsoft.com/dotnet/api/system.collections.generic.ienumerator-1.current?view=net-7.0)
60016001
// so we should be able to just return value here, and we could get rid of the
60026002
// complete variable which would be faster
@@ -6053,8 +6053,8 @@ namespace Microsoft.FSharp.Core
60536053
let mutable value = n - GenericOne
60546054

60556055
let inline current () =
6056-
// according to IEnumerator<int>.Current documentation, the result of of Current
6057-
// is undefined prior to the first call of MoveNext and post called to MoveNext
6056+
// according to IEnumerator<int>.Current documentation, the result of Current
6057+
// is undefined prior to the first call of MoveNext and post call to MoveNext
60586058
// that return false (see https://learn.microsoft.com/dotnet/api/system.collections.generic.ienumerator-1.current?view=net-7.0)
60596059
// so we should be able to just return value here, which would be faster
60606060
let derefValue = value

0 commit comments

Comments
 (0)