fsharp icon indicating copy to clipboard operation
fsharp copied to clipboard
verified publisher icon dotnet

Remove unused FCS support for compiling from ASTs

Open dsyme opened this issue 3 years ago • 4 comments

  1. FCS has an ability to take SyntaxTree as inputs instead of text. It was added by @eiriktsarpalis way back, but is to my knowledge entirely unused today - especially given the rate of churn in the specification of SyntaxTree as trivia has been added and other things cleaned up. I propose we remove this for simplicity's sake. At some later point we can take parsed inputs as input but there is no value in it at the moment.

  2. FCS also has the ability to create dynamic assemblies using Reflection.Emit, using the ilreflect.fs writer, including .NET Framework collectible assemblies. My proposal is we should also remove this, now that FSI has moved to using ilwrite.fs and ilreflect is now deprecated (and can't do things like emit debug symbols). Removing this would clean up a lot of entry point code.

dsyme avatar Jun 03 '22 12:06 dsyme

  1. Was used by the quotation compiler
  2. Was used by the stuff I wrote at xamarin for F# playgrounds as well as Edge.fs

All of these are pretty old now, interesting for talking points and the F# annals but all have lead to other projects now. The quotation compiler might still be used in the Linq optimiser lib that @eiriktsarpalis did too though.

7sharp9 avatar Jun 03 '22 17:06 7sharp9

It's used by the QuotationCompiler library, but that being said, I never got the chance to fully port that to .NET Core or more recent FSCS versions.

eiriktsarpalis avatar Jun 06 '22 11:06 eiriktsarpalis

Are both points approved to be removed? I would assume none of the (older) usages mentioned here are considered to be showstoppers as of today. And reducing the entry points surface would help the FCS codebase.

=> I am happy to proceed on this one if nobody gets hurt in the process

T-Gro avatar Sep 22 '22 18:09 T-Gro

Are both points approved to be removed? I would assume none of the (older) usages mentioned here are considered to be showstoppers as of today. And reducing the entry points surface would help the FCS codebase.

=> I am happy to proceed on this one if nobody gets hurt in the process

I think so, it's safe to remove them.

vzarytovskii avatar Sep 22 '22 20:09 vzarytovskii

Even with those two removals, the legacy // default for .NET framework // way of emitting for FSI purposes will remain. fsi --multiemit being disabled (= legacy single emit) [https://github.com/dotnet/fsharp/blob/main/docs/fsi-emit.md ]

That means some of the code doing ilreflect dynamic regeneration still has to exist. Unless we find a way of removing that as well? That would involve changing the default for .NETFramework scripting, which might be a breaking change for interactive sessions split into multiple fragments.

Is there a roadmap until which version we should support interactive .NETFramework scripting? @dsyme ; @vzarytovskii

T-Gro avatar Sep 23 '22 08:09 T-Gro

For now, I will have to keep ilreflect code related to FSI output (SingleAssembly-style) generation as long as Visual Studio remains netframework472. At least I can remove the other entry points mentioned in here, and ilreflect removal will have to wait for a little longer.

T-Gro avatar Sep 23 '22 10:09 T-Gro

Is there a roadmap until which version we should support interactive .NETFramework scripting?

Hard to say, and definitely orthogonal. @KevinRansom has a plan to effectively create final archived binaries for .NET Framework scripting but I think we're some way from executing on that

At least I can remove the other entry points mentioned in here, and ilreflect removal will have to wait for a little longer.

Yes, thanks

dsyme avatar Sep 23 '22 16:09 dsyme