Skip to content

Commit

Permalink
Fixes #17438 - Ensure that isinteractive multi-emit backing fields ar…
Browse files Browse the repository at this point in the history
…e not public (#17439)
  • Loading branch information
KevinRansom authored Jul 30, 2024
1 parent df43ab1 commit f6d21c2
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 39 deletions.
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/9.0.100.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@
* Treat `{ new Foo() }` as `SynExpr.ObjExpr` ([PR #17388](https://github.com/dotnet/fsharp/pull/17388))
* Optimize metadata reading for type members and custom attributes. ([PR #17364](https://github.com/dotnet/fsharp/pull/17364))
* 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))

### Breaking Changes
17 changes: 2 additions & 15 deletions src/Compiler/CodeGen/IlxGen.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
]
Expand Down Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions src/Compiler/Driver/OptimizeInputs.fs
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,6 @@ let ApplyAllOptimizations
importMap,
prevFile.FirstLoopRes.OptEnv,
isIncrementalFragment,
tcConfig.fsiMultiAssemblyEmit,
tcConfig.emitTailcalls,
prevFile.FirstLoopRes.HidingInfo,
file
Expand Down Expand Up @@ -436,7 +435,6 @@ let ApplyAllOptimizations
importMap,
prevFile.OptEnvExtraLoop,
isIncrementalFragment,
tcConfig.fsiMultiAssemblyEmit,
tcConfig.emitTailcalls,
prevPhase.FirstLoopRes.HidingInfo,
file
Expand Down Expand Up @@ -507,7 +505,6 @@ let ApplyAllOptimizations
importMap,
prevFile.OptEnvFinalSimplify,
isIncrementalFragment,
tcConfig.fsiMultiAssemblyEmit,
tcConfig.emitTailcalls,
prevPhase.FirstLoopRes.HidingInfo,
file
Expand Down
7 changes: 6 additions & 1 deletion src/Compiler/Interactive/fsi.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 5 additions & 18 deletions src/Compiler/Optimize/Optimizer.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -4361,35 +4360,23 @@ 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
// This optimizes and builds minfo w.r.t. the signature
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)

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
Expand All @@ -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 }
Expand Down
1 change: 0 additions & 1 deletion src/Compiler/Optimize/Optimizer.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ val internal OptimizeImplFile:
Import.ImportMap *
IncrementalOptimizationEnv *
isIncrementalFragment: bool *
fsiMultiAssemblyEmit: bool *
emitTailcalls: bool *
SignatureHidingInfo *
CheckedImplFile ->
Expand Down
37 changes: 36 additions & 1 deletion tests/FSharp.Compiler.ComponentTests/Scripting/Interactive.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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`` =
Expand Down Expand Up @@ -89,3 +89,38 @@ module ``External FSI tests`` =
|> runFsi
|> shouldSucceed


module MultiEmit =

[<Theory>]
[<InlineData(true)>]
[<InlineData(false)>]
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)

0 comments on commit f6d21c2

Please sign in to comment.