FCS Design Review Discussion Thread

[dsyme]

PRs #10756 and #10772 have done a lot of work to remove the "stuff that should never have been public" from FSharp.Compiler.Service.dll, down to about 40% of the previous surface area.

I'd like to now for us all collectively do a design review on the remaining surface area. There is no huge rush with this and this is an open discussion thread which I'll contribute to as we go

The remaining surface area is shown in:

1. the generated surface area baseline file (NOTE: if you're reading this before Further work on FCS API #10772 goes in then look here instead).

2. The various signature files

The public surface area is currently in these namespaces:

• FSharp.Compiler.SourceCodeServices.* (analysis, compilation, tooling, lexing)
• FSharp.Compiler.Interactive.Shell.* (scripting support)
• FSharp.Compiler.AbstractIL.* (for ILAssemblyReader hook for Rider)
• FSharp.Compiler.SyntaxTree.* (direct access to full untyped tree)

There is now essentially nothing public directly in FSharp.Compiler.*

My overall aim so far has been to get to the point were we can start an intentional community design process of looking at the remaining public contents and deciding what we want to do with renaming, organising into namespaces, deprecating etc. We can compare with Roslyn too.

One thing to note is that the design is fairly "first order" - there are relatively few public interfaces and relatively few hooks, and those that are don't have very complex or open semantics. This is important for lots of reasons.

By far the most brittle part of the design is SyntaxTree. This is because it is a big discriminated union. We should consider internalising the representation of these types and have FCS users match them with active patterns instead. This is a big but routine thing to implement.

Open discussion below. I'll edit this head issue or an RFC as decisions get made.

[dsyme]

@cartermp says

@dsyme For small additions it's probably not an issue, but eventually we'll want to do something about the fact that we expose our tree and that any additions to it will break all consumers. And we definitely want to make additions to it over time.

As mentioned above in the discussion - I think we should likely hide the representation of the major SyntaxTree types (especially SynExpr, SynType) and make FCS users use active patterns to access these. We've used this technique for F# quotations (FSharp.Core) and FSharpExpr (FCS) and it's allowed both to basically be binary compatible (or non-breaking for FSharpExpr) over many years of progressive updates. There is a slight performance cost to doing this (extra wrapper allocation objects on traversal) but there are techniques to minimise these and we could consider some further language work if it's a huge problem (I honestly don't expect it will be - there are already many allocations in most of our traversals, and the most important ones are internalised anyway and can use the internal access to the discriminated unions).

[cartermp]

I'd be happy to explore what the right "shape" or "shapes" are here. In #10758 and other work to spike on adding refactoring that one of the problems we have today is that we're simply missing some information in our trees.

A big difference between our current model and a model we may wish to go to is the notion of keeping absolution positions in a tree vs. simply computing the positions on-demand (like Roslyn). The latter makes manipulating the tree representing someone's code simpler and more performant, since it holds less information and we don't have to recompute the rest of a tree whenever an update is made.

Of course a change like that would be massively breaking for all F# applications that touch the tree. But it feels like a state where we'd like to eventually be.

[dsyme]

Yes there's two related topics here

1. how to hide the representation of SyntaxTree (assuming the future trajectory is that adding new constructs along the same lines as today)
2. how to re-engineer SyntaxTree to be better for source code manipulations

I can do (1) and I think it's necessary

(2) is a big topic where we really do need to draw on Roslyn. It's important to estimate if doing (2) later can still preserve the the pattern matching API to access the tree implemented in (1) or if it needs a fundamental rethink.

keeping absolution positions in a tree vs. simply computing the positions on-demand (like Roslyn).

Does this refer to range values in the tree? And does it require having all token information integrated in the tree? Please add links to the Roslyn data structure if you can

[smoothdeveloper]

@dsyme what makes (1) something (sounding) to be tackled as a priority while such work could benefit from insights gleaned by prototyping / tinkering more on (2)?

(1) will tend to reduce the breakage of code consuming the tree (great), but at the same time, that same code can already apply techniques reducing such breakage :

• in pattern match on DU, don't use positional but rather the named deconstruction syntax
• for frequent patterns, define own active pattern

Doesn't that provide most of "ease of upgrade" to consider it is bearable for some more time for the tooling to do like so far?

I do concur with the vision that all tooling working on a representation that doesn't break on upgrades is necessary to have a more consistent experience across tooling and ultimately a richer ecosystem.

But, I believe getting more insight on what would be ideal for (2) may be restricted in terms of design choice if we only think in terms of "making FCS binary compatible" without exploring more the choice made in Roslyn and taking more time to consider other design strategies.

Those design choices may impact how nice the end API in terms of manipulating the tree.

[dsyme]

@smoothdeveloper F# Analyzer support is not really possible without a binary compatible view of at least SyntaxTree, Symbols and Expr (that view could be a separate API to FCS though). Analyzers are necessarily binary components built against these APIs.

Tbh I'd imagine hiding representations will only make it easier to make design improvements like (2).

The adhoc techniques help in the current situation but are also problematic because they depend on access to DU representation, kind of making things worse.

[cartermp]

@dsyme yes, the Roslyn tree has a way to get the tokens for a node in the tree. Each token has a character width, as does the node it is a part of. You can then get a span (range) of a given token or node but it's computed on the spot rather than kept in the tree itself. That way there's no complexity in maintaining absolute positions when you modify the tree representing the source text. There's then a nice API to extract a source text from a syntax tree, and there is also a way to get the syntax tree from a given source text. Many more details that make this up, but I sort of view that as the key pieces that unblock several classes of refactorings for us.

[auduchinok]

Just in case, we use SyntaxTree a lot and heavily depend on its representation details. Please, if making a stable abstraction over it, move it to a namespace that isn't considered a stable API and don't make it internal.

[dsyme]

This is possible, though ultimately I believe we need to make FCS a binary compatible API.

.NET has no notion of binary compatibility that's partial, though one can invent such a notion I guess.

In practice active patterns are very close to sufficient for maintaining almost completely identical access to these kinds of data structures. The cost of APs is an allocation of two on deconstruction, and we could likely remove that cost by a language feature allowing case-based deconstruction via a Tag/case pattern (instead of by producing a Choice value using an active pattern.)

So I'm optimistic we can both hide the representations and give essentially identical access to today.

To be clear having direct access to representations of types is devastating for the long term maintenance of a library (especially if there's any desire to be binary compatible).

[vzarytovskii]

@dsyme @cartermp I think it's worth it having dedicated design discussion session(s) with community eventually, where we can collect feedback and talk about the changes we plan. What do you think?