Skip to content

SourceTree: give every node a real source location (revive #178) #342

Description

@amis92

Captures the design from #178 (feature/sourcetree, July 2023), which is being closed unmerged. Nothing tracked it until now.

The idea

Make every SourceNode able to answer "where did I come from?", so diagnostics can point at a position in the original datafile instead of just naming a symbol.

The approach is Roslyn's, adapted: rather than tracking character offsets through XML parsing, each node occupies a contiguous span in an abstract node-index space, and spans are composed bottom-up.

  1. Generated, cached span arithmetic. NodeCore gains

    public int GetSpanLength()
    {
        if (spanLength > 0) return spanLength;
        return spanLength = 1 + CalculateDescendantSpanLength();
    }
    protected abstract int CalculateDescendantSpanLength();

    with CalculateDescendantSpanLength() emitted per core type by a new CoreSpanCalculationPartialGenerator, summing children's lengths. Computed once per node, cached.

  2. Nodes know their tree. SourceNode gains SourceTree? Tree => treeCore ??= Parent?.Tree, plus a thread-safe WithTree(tree) that associates a root with a tree via Interlocked.CompareExchange, cloning when the node is already attached elsewhere.

  3. A location model. Text/ primitives — TextSpan, LinePosition, LinePositionSpan, FileLinePositionSpan, Location, LocationKind, NoLocation, SourceLocation.

  4. Lazy trees over datafiles. IDatafileInfo.CreateTree() and LazyDatafileSourceTree, so a workspace can hand out trees without eagerly parsing.

  5. Incidental: XmlWorkspaceOptions.IncludeUnknown so non-data files are not loaded into a workspace by default.

Why this still matters

The roster-engine stack (#317#322) independently re-created the Text/ primitives under WarHub.ArmouryModel.Extensions/Text/ — but only the types. The mechanism that gives them real values was never built, and the gap is load-bearing today. SourceNodeExtensions on main is four TODOs in a trenchcoat:

public static Location GetLocation(this SourceNode node)
{
    // TODO remove this after SourceNode has this method itself
    // we don't have tree ref here
    var tree = SourceTree.CreateForRoot(node.AncestorsAndSelf().Last());
    return new SourceLocation(tree, node.GetSpan());
}

public static int GetWidth(this SourceNode node)
{
    // TODO remove this after SourceNode has this method itself
    return node.DescendantsAndSelf().Count();
}

Each item maps onto a piece of #178:

TODO on main What #178 provides
GetLocation — "we don't have tree ref here" SourceNode.Tree
GetSourceTree — "should not require compilation (a node knows its tree)" SourceNode.Tree
GetWidth — full DescendantsAndSelf().Count() per call generated + cached GetSpanLength()
GetSpan — sums preceding-sibling widths up the ancestor chain span arithmetic composed from cached lengths

Consequences as it stands:

  • Locations are effectively fictional. SourceTree.GetLineSpan is return default; // TODO implement, and GetLocation fabricates a throwaway InMemoryTree with no FilePath. So a Location cannot name a file, line or column — it is a span into an anonymous in-memory tree.
  • It is recomputed the hard way, every time. GetSpan calls GetWidth for every preceding sibling at every ancestor level, and each GetWidth is a full subtree traversal. Cost grows with everything preceding the node in document order, with no caching, and a fresh tree object is allocated per call. Binder and the diagnostic bags call node.GetLocation() at dozens of sites.

This is the difference between "wham can tell you a constraint is unsatisfiable" and "wham can tell you it is unsatisfiable at Catalogue.cat line 412".

Reviving it

The branch is less stale than its age suggests — it conflicts with main in only two files, XmlDocument.cs and XmlFileExtensions.cs, both touched by the zipped-datafile fix (#324).

The real design question is not the merge, it is where the location model should live. #178 put Text/ in WarHub.ArmouryModel.Source (the shipped package, next to the nodes); the roster-engine stack put its copies in WarHub.ArmouryModel.Extensions. The two implementations are near-copies but not identical (TextSpan differs by a few lines). Whichever is kept, the duplicate should go — spans belong wherever SourceNode can reach them, which argues for Source.

Also worth deciding whether node-index spans are the right currency, or whether real character offsets from the XML reader would be more useful for editor scenarios.

Related: #334 (tracks the GetLineSpan TODO among other incomplete work), #281 and #282 (use SourceTree as document identity for incremental compilation and the workspace — consumers of this, not substitutes for it).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions