Skip to content

Commit

Permalink
Fix 16105
Browse files Browse the repository at this point in the history
  • Loading branch information
KevinRansom committed Feb 20, 2024
1 parent 9d36d64 commit 889f828
Show file tree
Hide file tree
Showing 7 changed files with 238 additions and 24 deletions.
27 changes: 21 additions & 6 deletions src/Compiler/Optimize/Optimizer.fs
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,10 @@ let GetInfoForVal cenv env m (vref: ValRef) =
GetInfoForLocalValue cenv env vref.binding m
else
GetInfoForNonLocalVal cenv env vref
res

let GetInfoForValWithCheck cenv env m (vref: ValRef) =
let res = GetInfoForVal cenv env m vref
check vref res |> ignore
res

Expand Down Expand Up @@ -3062,11 +3065,14 @@ and TryOptimizeVal cenv env (vOpt: ValRef option, mustInline, inlineIfLambda, va
failwith "tuple, union and record values cannot be marked 'inline'"

| UnknownValue when mustInline ->
warning(Error(FSComp.SR.optValueMarkedInlineHasUnexpectedValue(), m)); None
warning(Error(FSComp.SR.optValueMarkedInlineHasUnexpectedValue(), m))
None

| _ when mustInline ->
warning(Error(FSComp.SR.optValueMarkedInlineCouldNotBeInlined(), m)); None
| _ -> None
warning(Error(FSComp.SR.optValueMarkedInlineCouldNotBeInlined(), m))
None

| _ -> None

and TryOptimizeValInfo cenv env m vinfo =
if vinfo.HasEffect then None else TryOptimizeVal cenv env (None, false, false, vinfo.Info, m)
Expand All @@ -3089,7 +3095,7 @@ and OptimizeVal cenv env expr (v: ValRef, m) =

let g = cenv.g

let valInfoForVal = GetInfoForVal cenv env m v
let valInfoForVal = GetInfoForValWithCheck cenv env m v

match TryOptimizeVal cenv env (Some v, v.MustInline, v.InlineIfLambda, valInfoForVal.ValExprInfo, m) with
| Some e ->
Expand Down Expand Up @@ -3402,7 +3408,7 @@ and TryInlineApplication cenv env finfo (tyargs: TType list, args: Expr list, m)
| _ -> false
| _ -> false
| _ -> false
| _ -> false
| _ -> false

if isValFromLazyExtensions then None else

Expand All @@ -3421,6 +3427,15 @@ and TryInlineApplication cenv env finfo (tyargs: TType list, args: Expr list, m)

if isGetHashCode then None else

let isApplicationComplete =
match finfo.Info with
| ValValue (_, CurriedLambdaValue (_, _, _, Expr.Lambda (_, _, _, _, Expr.App (Expr.Val(vr,_,_), _, _, _, _), _, _), _) ) ->
IsPartialExprVal (GetInfoForVal cenv env m vr).ValExprInfo
| _ ->
false

if isApplicationComplete then None else

// Inlining lambda
let f2R = CopyExprForInlining cenv false f2 m

Expand Down Expand Up @@ -3598,7 +3613,7 @@ and OptimizeApplication cenv env (f0, f0ty, tyargs, args, m) =
// Note we also have to check the argument count to ensure this is a direct call (or a partial application).
let doesNotMakeCriticalTailcall =
vref.MakesNoCriticalTailcalls ||
(let valInfoForVal = GetInfoForVal cenv env m vref in valInfoForVal.ValMakesNoCriticalTailcalls) ||
(let valInfoForVal = GetInfoForValWithCheck cenv env m vref in valInfoForVal.ValMakesNoCriticalTailcalls) ||
(match env.functionVal with | None -> false | Some (v, _) -> valEq vref.Deref v)
if doesNotMakeCriticalTailcall then
let numArgs = otherArgs.Length + newArgs.Length
Expand Down
4 changes: 2 additions & 2 deletions tests/AheadOfTime/Trimming/check.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ function CheckTrim($root, $tfm, $outputfile, $expected_len) {
# error NETSDK1124: Trimming assemblies requires .NET Core 3.0 or higher.

# Check net7.0 trimmed assemblies
CheckTrim -root "SelfContained_Trimming_Test" -tfm "net8.0" -outputfile "FSharp.Core.dll" -expected_len 287232
CheckTrim -root "SelfContained_Trimming_Test" -tfm "net8.0" -outputfile "FSharp.Core.dll" -expected_len 303104

# Check net7.0 trimmed assemblies
CheckTrim -root "StaticLinkedFSharpCore_Trimming_Test" -tfm "net8.0" -outputfile "StaticLinkedFSharpCore_Trimming_Test.dll" -expected_len 8821248
CheckTrim -root "StaticLinkedFSharpCore_Trimming_Test" -tfm "net8.0" -outputfile "StaticLinkedFSharpCore_Trimming_Test.dll" -expected_len 8839680
11 changes: 2 additions & 9 deletions tests/FSharp.Compiler.ComponentTests/Debugger/PortablePdbs.fs
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,7 @@ module Baz =
Line 16, Col 20, Line 16, Col 22
Line 21, Col 20, Line 21, Col 22
]
VerifyDocuments [
Path.Combine(Environment.CurrentDirectory, "test.fs")
]
VerifyDocuments [ "test.fs" ]
]

[<Fact>]
Expand All @@ -100,9 +98,4 @@ module M =
|> withPortablePdb
|> compile
|> shouldSucceed
|> verifyPdb [
VerifyDocuments [
Path.Combine(Environment.CurrentDirectory, "test.fsi")
Path.Combine(Environment.CurrentDirectory, "test.fs")
]
]
|> verifyPdb [ VerifyDocuments [ "test.fsi"; "test.fs" ] ]
201 changes: 200 additions & 1 deletion tests/FSharp.Compiler.ComponentTests/EmittedIL/NoCompilerInlining.fs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,206 @@ namespace EmittedIL
open Xunit
open FSharp.Test.Compiler

module ``NoCompilerInlining`` =
module NoCompilerInlining =

[<Fact>]
let ``Methods marked internal not available for cross module inlining``() =

let outerModule =
FSharpWithFileName
"outerModule.fs"
"""
module internal OuterModule
open System.Runtime.CompilerServices
[<assembly:InternalsVisibleTo("middleModule")>]
do ()
let sayOuterModuleHello () = System.Console.WriteLine("Hello World") """
|> withOptimize
|> asLibrary
|> withName "outerLibrary"

let middleModule =
FSharpWithFileName
"middleModule.fs"
"""
module MiddleModule
let sayMiddleModuleHello () = OuterModule.sayOuterModuleHello()"""
|> withOptimize
|> withReferences [outerModule]
|> asLibrary
|> withName "middleModule"

FSharpWithFileName
"program.fs"
"""MiddleModule.sayMiddleModuleHello()"""
|> withOptimize
|> withReferences [middleModule; outerModule]
|> withName "Program"
|> compileExeAndRun
|> shouldSucceed
|> verifyIL [ """
.method public static void main@() cil managed
{
.entrypoint
.maxstack 8
IL_0000: call void [middleModule]MiddleModule::sayMiddleModuleHello()
IL_0005: ret
}
""" ]

[<Fact>]
let ``Methods marked public available for cross module inlining``() =

let outerModule =
FSharpWithFileName
"outerModule.fs"
"""
module OuterModule
open System.Runtime.CompilerServices
let sayOuterModuleHello () = System.Console.WriteLine("Hello World") """
|> withOptimize
|> asLibrary
|> withName "outerLibrary"

let middleModule =
FSharpWithFileName
"middleModule.fs"
"""
module MiddleModule
let sayMiddleModuleHello () = OuterModule.sayOuterModuleHello()"""
|> withOptimize
|> withReferences [outerModule]
|> asLibrary
|> withName "middleModule"

FSharpWithFileName
"program.fs"
"""MiddleModule.sayMiddleModuleHello()"""
|> withOptimize
|> withReferences [middleModule; outerModule]
|> withName "Program"
|> compileExeAndRun
|> shouldSucceed
|> verifyIL [ """
.method public static void main@() cil managed
{
.entrypoint
.maxstack 8
IL_0000: ldstr "Hello World"
IL_0005: call void [runtime]System.Console::WriteLine(string)
IL_000a: ret
}
""" ]


[<Fact>]
let ``Nested Module marked internal not available for cross module inlining``() =

let outerModule =
FSharpWithFileName
"outerModule.fs"
"""
module OuterModule
open System.Runtime.CompilerServices
[<assembly:InternalsVisibleTo("middleModule")>]
do ()
module internal nestedModule =
let sayNestedModuleHello () = System.Console.WriteLine("Hello World")
let sayOuterModuleHello () = nestedModule.sayNestedModuleHello () """
|> withOptimize
|> asLibrary
|> withName "outerLibrary"

let middleModule =
FSharpWithFileName
"middleModule.fs"
"""
module MiddleModule
let sayMiddleModuleHello () = OuterModule.sayOuterModuleHello()"""
|> withOptimize
|> withReferences [outerModule]
|> asLibrary
|> withName "middleModule"

FSharpWithFileName
"program.fs"
"""MiddleModule.sayMiddleModuleHello()"""
|> withOptimize
|> withReferences [middleModule; outerModule]
|> withName "Program"
|> compileExeAndRun
|> shouldSucceed
|> verifyIL [ """
.method public static void main@() cil managed
{
.entrypoint
.maxstack 8
IL_0000: ldstr "Hello World"
IL_0005: call void [runtime]System.Console::WriteLine(string)
IL_000a: ret
}
""" ]

[<Fact>]
let ``Nested Module marked public available for cross module inlining``() =

let outerModule =
FSharpWithFileName
"outerModule.fs"
"""
module OuterModule
open System.Runtime.CompilerServices
module nestedModule =
let sayNestedModuleHello () = System.Console.WriteLine("Hello World")
let sayOuterModuleHello () = nestedModule.sayNestedModuleHello () """
|> withOptimize
|> asLibrary
|> withName "outerLibrary"

let middleModule =
FSharpWithFileName
"middleModule.fs"
"""
module MiddleModule
let sayMiddleModuleHello () = OuterModule.sayOuterModuleHello()"""
|> withOptimize
|> withReferences [outerModule]
|> asLibrary
|> withName "middleModule"

FSharpWithFileName
"program.fs"
"""MiddleModule.sayMiddleModuleHello()"""
|> withOptimize
|> withReferences [middleModule; outerModule]
|> withName "Program"
|> compileExeAndRun
|> shouldSucceed
|> verifyIL [ """
.method public static void main@() cil managed
{
.entrypoint
.maxstack 8
IL_0000: ldstr "Hello World"
IL_0005: call void [runtime]System.Console::WriteLine(string)
IL_000a: ret
}
""" ]



[<Fact>]
let ``Function marked with NoCompilerInlining is not inlined by the compiler``() =
FSharp """
Expand Down
11 changes: 8 additions & 3 deletions tests/FSharp.Test.Utilities/Compiler.fs
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,10 @@ module rec Compiler =
let FSharp (source: string) : CompilationUnit =
Fs source

let FSharpWithFileName name (source: string) : CompilationUnit =
fsFromString (SourceCodeFileKind.Fs({FileName=name; SourceText=Some source }))
|> FS

let FsFromPath (path: string) : CompilationUnit =
fsFromString (SourceFromPath path)
|> FS
Expand Down Expand Up @@ -1362,12 +1366,13 @@ Actual:
if documents <> expectedDocuments then
failwith $"Expected documents are different from PDB.\nExpected: %A{expectedDocuments}\nActual: %A{documents}"

let private verifyPdbOptions reader options =
let private verifyPdbOptions optOutputPath reader options =
let outputPath = Path.GetDirectoryName(optOutputPath |> Option.defaultValue ".")
for option in options do
match option with
| VerifyImportScopes scopes -> verifyPdbImportTables reader scopes
| VerifySequencePoints sp -> verifySequencePoints reader sp
| VerifyDocuments docs -> verifyDocuments reader docs
| VerifyDocuments docs -> verifyDocuments reader (docs |> List.map(fun doc -> Path.Combine(outputPath, doc)))
| _ -> failwith $"Unknown verification option: {option.ToString()}"

let private verifyPortablePdb (result: CompilationOutput) options : unit =
Expand All @@ -1386,7 +1391,7 @@ Actual:
| _ -> failwith "Only F# compilations are supported when verifying PDBs."

verifyPdbFormat reader compilationType
verifyPdbOptions reader options
verifyPdbOptions result.OutputPath reader options
| _ -> failwith "Output path is not set, please make sure compilation was successfull."

()
Expand Down
7 changes: 4 additions & 3 deletions tests/FSharp.Test.Utilities/CompilerAssert.fs
Original file line number Diff line number Diff line change
Expand Up @@ -439,9 +439,10 @@ module rec CompilerAssertHelpers =
| Some text ->
// In memory source file copy it to the build directory
let source = item.ChangeExtension
File.WriteAllText (source.GetSourceFileName, text)
disposals.Add(disposeFile source.GetSourceFileName)
yield source
let destFileName = Path.Combine(outputDirectory.FullName, Path.GetFileName(source.GetSourceFileName))
File.WriteAllText (destFileName, text)
disposals.Add(disposeFile destFileName)
yield source.WithFileName(destFileName)
| None ->
// On Disk file
let sourceFileName = item.GetSourceFileName
Expand Down
1 change: 1 addition & 0 deletions vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<OutputType>Library</OutputType>
<NoWarn>$(NoWarn);75</NoWarn>
<NoWarn>$(NoWarn);44</NoWarn><!-- warning about Roslyn API only for F# and TypeScript -->
<NoWarn>$(NoWarn);FS3511</NoWarn> <!-- This state machine is not statically compilable.@@@@ Do we really need this -->
<DisableImplicitFSharpCoreReference>true</DisableImplicitFSharpCoreReference>
<OtherFlags>$(OtherFlags) --warnon:1182 --subsystemversion:6.00</OtherFlags>
<EnableDefaultEmbeddedResourceItems>false</EnableDefaultEmbeddedResourceItems>
Expand Down

0 comments on commit 889f828

Please sign in to comment.