From fe5df6441564a231769361b849059ab401ab3de6 Mon Sep 17 00:00:00 2001 From: KevinRansom Date: Tue, 23 Jul 2024 22:36:33 -0700 Subject: [PATCH 1/4] Initial --- .../.FSharp.Compiler.Service/9.0.100.md | 2 ++ src/Compiler/CodeGen/IlxGen.fs | 17 ++--------------- src/Compiler/Interactive/fsi.fs | 7 ++++++- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/9.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/9.0.100.md index 4888bd1aad6..8033401aa42 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/9.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/9.0.100.md @@ -11,5 +11,7 @@ * Change compiler default setting realsig+ when building assemblies ([Issue #17384](https://github.com/dotnet/fsharp/issues/17384), [PR #17378](https://github.com/dotnet/fsharp/pull/17385)) * Change compiler default setting for compressedMetadata ([Issue #17379](https://github.com/dotnet/fsharp/issues/17379), [PR #17383](https://github.com/dotnet/fsharp/pull/17383)) * Enforce `AttributeTargets` on unions. ([PR #17389](https://github.com/dotnet/fsharp/pull/17389)) +* Ensure that isinteractive multi-emit backing fields are not public. ([Issue #17439](https://github.com/dotnet/fsharp/issues/17438)), ([PR #17439](https://github.com/dotnet/fsharp/pull/17439)) +https://github.com/dotnet/fsharp/issues/17438 ### Breaking Changes diff --git a/src/Compiler/CodeGen/IlxGen.fs b/src/Compiler/CodeGen/IlxGen.fs index 4c12b687a08..f182639ae1c 100644 --- a/src/Compiler/CodeGen/IlxGen.fs +++ b/src/Compiler/CodeGen/IlxGen.fs @@ -2082,14 +2082,7 @@ type AnonTypeGenerationTable() = mkILFields [ for _, fldName, fldTy in flds -> - - let access = - if cenv.options.isInteractive && cenv.options.fsiMultiAssemblyEmit then - ILMemberAccess.Public - else - ILMemberAccess.Private - - let fdef = mkILInstanceField (fldName, fldTy, None, access) + let fdef = mkILInstanceField (fldName, fldTy, None, ILMemberAccess.Private) let attrs = [ g.CompilerGeneratedAttribute; g.DebuggerBrowsableNeverAttribute ] fdef.With(customAttrs = mkILCustomAttrs attrs) ] @@ -11059,13 +11052,7 @@ and GenTypeDef cenv mgbuf lazyInitInfo eenv m (tycon: Tycon) : ILTypeRef option // The IL field is hidden if the property/field is hidden OR we're using a property // AND the field is not mutable (because we can take the address of a mutable field). // Otherwise fields are always accessed via their property getters/setters - // - // Additionally, don't hide fields for multiemit in F# Interactive - let isFieldHidden = - isPropHidden - || (not useGenuineField - && not isFSharpMutable - && not (cenv.options.isInteractive && cenv.options.fsiMultiAssemblyEmit)) + let isFieldHidden = isPropHidden || (not useGenuineField && not isFSharpMutable) let extraAttribs = match tyconRepr with diff --git a/src/Compiler/Interactive/fsi.fs b/src/Compiler/Interactive/fsi.fs index c27382e0963..7e42d1d57e2 100644 --- a/src/Compiler/Interactive/fsi.fs +++ b/src/Compiler/Interactive/fsi.fs @@ -1861,7 +1861,12 @@ type internal FsiDynamicCompiler let asm = match opts.pdbfile, pdbBytes with - | (Some pdbfile), (Some pdbBytes) -> File.WriteAllBytes(pdbfile, pdbBytes) + | (Some pdbfile), (Some pdbBytes) -> + File.WriteAllBytes(pdbfile, pdbBytes) +#if FOR_TESTING + Directory.CreateDirectory(scriptingSymbolsPath.Value) |> ignore + File.WriteAllBytes(Path.ChangeExtension(pdbfile, ".dll"), assemblyBytes) +#endif | _ -> () match pdbBytes with From 2f654f38aed0f1c916bd287ad4dc3bd79825e029 Mon Sep 17 00:00:00 2001 From: KevinRansom Date: Wed, 24 Jul 2024 01:59:36 -0700 Subject: [PATCH 2/4] more cleanup --- src/Compiler/Driver/OptimizeInputs.fs | 3 --- src/Compiler/Optimize/Optimizer.fs | 23 +++++------------------ src/Compiler/Optimize/Optimizer.fsi | 1 - 3 files changed, 5 insertions(+), 22 deletions(-) diff --git a/src/Compiler/Driver/OptimizeInputs.fs b/src/Compiler/Driver/OptimizeInputs.fs index 85cde3b6c0e..36eae5734ce 100644 --- a/src/Compiler/Driver/OptimizeInputs.fs +++ b/src/Compiler/Driver/OptimizeInputs.fs @@ -389,7 +389,6 @@ let ApplyAllOptimizations importMap, prevFile.FirstLoopRes.OptEnv, isIncrementalFragment, - tcConfig.fsiMultiAssemblyEmit, tcConfig.emitTailcalls, prevFile.FirstLoopRes.HidingInfo, file @@ -436,7 +435,6 @@ let ApplyAllOptimizations importMap, prevFile.OptEnvExtraLoop, isIncrementalFragment, - tcConfig.fsiMultiAssemblyEmit, tcConfig.emitTailcalls, prevPhase.FirstLoopRes.HidingInfo, file @@ -507,7 +505,6 @@ let ApplyAllOptimizations importMap, prevFile.OptEnvFinalSimplify, isIncrementalFragment, - tcConfig.fsiMultiAssemblyEmit, tcConfig.emitTailcalls, prevPhase.FirstLoopRes.HidingInfo, file diff --git a/src/Compiler/Optimize/Optimizer.fs b/src/Compiler/Optimize/Optimizer.fs index bc4c0829871..5140a3da043 100644 --- a/src/Compiler/Optimize/Optimizer.fs +++ b/src/Compiler/Optimize/Optimizer.fs @@ -4348,8 +4348,7 @@ and OptimizeModuleDefs cenv (env, bindInfosColl) defs = let defs, minfos = List.unzip defs (defs, UnionOptimizationInfos minfos), (env, bindInfosColl) -and OptimizeImplFileInternal cenv env isIncrementalFragment fsiMultiAssemblyEmit hidden implFile = - let g = cenv.g +and OptimizeImplFileInternal cenv env isIncrementalFragment hidden implFile = let (CheckedImplFile (qname, pragmas, signature, contents, hasExplicitEntryPoint, isScript, anonRecdTypes, namedDebugPointsForInlinedCode)) = implFile let env, contentsR, minfo, hidden = // FSI compiles interactive fragments as if you're typing incrementally into one module. @@ -4361,13 +4360,7 @@ and OptimizeImplFileInternal cenv env isIncrementalFragment fsiMultiAssemblyEmit // This optimizes and builds minfo ignoring the signature let (defR, minfo), (_env, _bindInfosColl) = OptimizeModuleContents cenv (env, []) contents let hidden = ComputeImplementationHidingInfoAtAssemblyBoundary defR hidden - let minfo = - // In F# interactive multi-assembly mode, no internals are accessible across interactive fragments. - // In F# interactive single-assembly mode, internals are accessible across interactive fragments. - if fsiMultiAssemblyEmit then - AbstractLazyModulInfoByHiding true hidden minfo - else - AbstractLazyModulInfoByHiding false hidden minfo + let minfo = AbstractLazyModulInfoByHiding false hidden minfo let env = BindValsInModuleOrNamespace cenv minfo env env, defR, minfo, hidden else @@ -4375,13 +4368,7 @@ and OptimizeImplFileInternal cenv env isIncrementalFragment fsiMultiAssemblyEmit let mexprR, minfo = OptimizeModuleExprWithSig cenv env signature contents let hidden = ComputeSignatureHidingInfoAtAssemblyBoundary signature hidden let minfoExternal = AbstractLazyModulInfoByHiding true hidden minfo - let env = - // In F# interactive multi-assembly mode, internals are not accessible in the 'env' used intra-assembly - // In regular fsc compilation, internals are accessible in the 'env' used intra-assembly - if g.isInteractive && fsiMultiAssemblyEmit then - BindValsInModuleOrNamespace cenv minfoExternal env - else - BindValsInModuleOrNamespace cenv minfo env + let env = BindValsInModuleOrNamespace cenv minfo env env, mexprR, minfoExternal, hidden let implFileR = CheckedImplFile (qname, pragmas, signature, contentsR, hasExplicitEntryPoint, isScript, anonRecdTypes, namedDebugPointsForInlinedCode) @@ -4389,7 +4376,7 @@ and OptimizeImplFileInternal cenv env isIncrementalFragment fsiMultiAssemblyEmit env, implFileR, minfo, hidden /// Entry point -let OptimizeImplFile (settings, ccu, tcGlobals, tcVal, importMap, optEnv, isIncrementalFragment, fsiMultiAssemblyEmit, emitTailcalls, hidden, mimpls) = +let OptimizeImplFile (settings, ccu, tcGlobals, tcVal, importMap, optEnv, isIncrementalFragment, emitTailcalls, hidden, mimpls) = let cenv = { settings=settings scope=ccu @@ -4404,7 +4391,7 @@ let OptimizeImplFile (settings, ccu, tcGlobals, tcVal, importMap, optEnv, isIncr realsig = tcGlobals.realsig } - let env, _, _, _ as results = OptimizeImplFileInternal cenv optEnv isIncrementalFragment fsiMultiAssemblyEmit hidden mimpls + let env, _, _, _ as results = OptimizeImplFileInternal cenv optEnv isIncrementalFragment hidden mimpls let optimizeDuringCodeGen disableMethodSplitting expr = let env = { env with disableMethodSplitting = env.disableMethodSplitting || disableMethodSplitting } diff --git a/src/Compiler/Optimize/Optimizer.fsi b/src/Compiler/Optimize/Optimizer.fsi index aa205b86221..17912af7598 100644 --- a/src/Compiler/Optimize/Optimizer.fsi +++ b/src/Compiler/Optimize/Optimizer.fsi @@ -85,7 +85,6 @@ val internal OptimizeImplFile: Import.ImportMap * IncrementalOptimizationEnv * isIncrementalFragment: bool * - fsiMultiAssemblyEmit: bool * emitTailcalls: bool * SignatureHidingInfo * CheckedImplFile -> From d285452d69e27405163570cdfe547ab5afd7c73d Mon Sep 17 00:00:00 2001 From: KevinRansom Date: Wed, 24 Jul 2024 04:15:42 -0700 Subject: [PATCH 3/4] Simple impl test --- .../Scripting/Interactive.fs | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/tests/FSharp.Compiler.ComponentTests/Scripting/Interactive.fs b/tests/FSharp.Compiler.ComponentTests/Scripting/Interactive.fs index 58f90304853..4fab401b9c6 100644 --- a/tests/FSharp.Compiler.ComponentTests/Scripting/Interactive.fs +++ b/tests/FSharp.Compiler.ComponentTests/Scripting/Interactive.fs @@ -3,9 +3,9 @@ namespace Scripting open Xunit + open System open FSharp.Test.Compiler -open FSharp.Compiler.Interactive.Shell open FSharp.Test.ScriptHelpers module ``Interactive tests`` = @@ -89,3 +89,38 @@ module ``External FSI tests`` = |> runFsi |> shouldSucceed + +module MultiEmit = + + [] + [] + [] + let ``FSharp record in script`` (useMultiEmit) = + + let args : string array = [| if useMultiEmit then "--multiemit+" else "--multiemit-"|] + use session = new FSharpScript(additionalArgs=args) + + let scriptIt submission = + + let result, errors = session.Eval(submission) + Assert.Empty(errors) + match result with + | Ok _ -> () + | _ -> Assert.True(false, $"Failed in line: {submission}") + + [| + """type R = { x: int }""" + """let a = { x = 7 } """ + """if a.x <> 7 then failwith $"1: Failed {a.x} <> 7" """ + """if a.x <> 7 then failwith $"2: Failed {a.x} <> 7" """ + """if a.x <> 7 then failwith $"3: Failed {a.x} <> 7" """ + """if a.x <> 7 then failwith $"4: Failed {a.x} <> 7" """ + """let b = { x = 9 }""" + """if a.x <> 7 then failwith $"5: Failed {a.x} <> 7" """ + """if b.x <> 9 then failwith $"6: Failed {b.x} <> 9" """ + """let A = {| v = 7.2 |}""" + """if A.v <> 7.2 then failwith $"7: Failed {A.v} <> 7.2" """ + """let B = {| v = 9.3 |}""" + """if A.v <> 7.2 then failwith $"8: Failed {A.v} <> 7.2" """ + """if B.v <> 9.3 then failwith $"9: Failed {A.v} <> 9.3" """ + |] |> Seq.iter(fun item -> item |> scriptIt) From 411d0aee5e93bdf35d8a12ac1c6a96ac1a891f20 Mon Sep 17 00:00:00 2001 From: Petr Date: Thu, 25 Jul 2024 13:00:26 +0200 Subject: [PATCH 4/4] Update 9.0.100.md --- docs/release-notes/.FSharp.Compiler.Service/9.0.100.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/9.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/9.0.100.md index 462f497f560..8d28649581f 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/9.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/9.0.100.md @@ -14,5 +14,4 @@ * Enforce `AttributeTargets` on unions. ([PR #17389](https://github.com/dotnet/fsharp/pull/17389)) * Ensure that isinteractive multi-emit backing fields are not public. ([Issue #17439](https://github.com/dotnet/fsharp/issues/17438)), ([PR #17439](https://github.com/dotnet/fsharp/pull/17439)) -https://github.com/dotnet/fsharp/issues/17438 ### Breaking Changes