mirror of https://github.com/icsharpcode/ILSpy.git
You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
166 lines
10 KiB
166 lines
10 KiB
export const meta = { |
|
name: 'phase-s-spikes', |
|
description: 'Phase S de-risking spikes (E1-E4) for the slot-based C# AST rewrite, in isolated worktrees, then synthesis', |
|
phases: [ |
|
{ title: 'Spikes', detail: 'E1 migration mechanic, E2 collection perf, E3 token-drop printer, E4 inheritance shape' }, |
|
{ title: 'Synthesis', detail: 'fold spike findings into the Phase 1/3 decisions' }, |
|
], |
|
} |
|
|
|
// Shared context handed to every spike: build discipline + branch state. |
|
// (The design doc ROLES_FREE_SLOT_AST_DESIGN.md is untracked, so worktrees won't have it; |
|
// each prompt carries the spike's relevant slice inline.) |
|
const COMMON = ` |
|
Repo: ICSharpCode ILSpy decompiler. You are on a fresh git worktree branched off commit 76a965ae2 |
|
("Drop dead IFreezable/Freeze apparatus from the C# AST"). The C# AST lives in |
|
ICSharpCode.Decompiler/CSharp/Syntax/ and currently uses the NRefactory role + doubly-linked-list |
|
child model (per-node firstChild/nextSibling, each child tagged with a Role; GetChildByRole is a |
|
linear scan). The goal of the larger rewrite is a slot-based model (fixed source-ordered slots, |
|
O(1) access) generated by the Roslyn incremental generator in |
|
ICSharpCode.Decompiler.Generators/DecompilerSyntaxTreeGenerator.cs. |
|
|
|
BUILD DISCIPLINE (critical on this box): |
|
- Prefix every dotnet build/test with OPENSSL_ENABLE_SHA1_SIGNATURES=1 |
|
- Pass -p:RestoreEnablePackagePruning=false on restore/build to avoid pruning packages.lock.json. |
|
- Do NOT commit packages.lock.json changes. Keep your prototype commits to source only. |
|
- The decompiler lib is single-target netstandard2.0, LangVersion 14. |
|
|
|
TEST SETUP (required before running ANY decompiler test in your worktree): |
|
- The ILSpy-tests submodule is uninitialized; the test harness needs <repo>/ILSpy-tests/nuget to |
|
exist (matrix compiler / reference-assembly cache) or every test Assert.Fails at fixture setup. |
|
Run once in your worktree root: ln -s /home/siegfried/Projects/ILSpy/ILSpy-tests/nuget ILSpy-tests/nuget |
|
- That cache is INCOMPLETE and there is NO network: matrix configs needing absent packages |
|
(UseRoslyn4_14_0, UseRoslynLatest, TargetNet40, ...) fail at COMPILE setup with a Path.Exists |
|
assert in RefAssembliesToolset.GetPath. Those failures are ENVIRONMENTAL, not decompiler bugs -- |
|
ignore them and focus on the toolset configs that actually compile (the default/cached compiler; |
|
~1195 cases run today). Filter your test runs accordingly and never attribute a Path.Exists / |
|
RefAssembliesToolset failure to a code change. |
|
|
|
This is a SPIKE: prototype only enough to answer the question with evidence. Commit your prototype |
|
in your worktree (so the diff is inspectable) but it will NOT be merged. Be concrete and quantitative. |
|
` |
|
|
|
const FINDINGS = { |
|
type: 'object', |
|
additionalProperties: false, |
|
required: ['spike', 'question', 'whatIDid', 'findings', 'recommendation', 'confidence'], |
|
properties: { |
|
spike: { type: 'string' }, |
|
question: { type: 'string', description: 'the decision this spike informs' }, |
|
whatIDid: { type: 'string' }, |
|
findings: { type: 'string', description: 'key observations, with file refs and numbers where relevant' }, |
|
recommendation: { type: 'string' }, |
|
confidence: { type: 'string', enum: ['low', 'medium', 'high'] }, |
|
metrics: { type: 'string', description: 'concrete numbers: LOC, diff size, perf timings, generated-code volume; "n/a" if none' }, |
|
prototypeCommit: { type: 'string', description: 'sha of the prototype commit in this worktree, or "none"' }, |
|
risksFound: { type: 'string', description: 'anything that surprised you or threatens the design' }, |
|
}, |
|
} |
|
|
|
const SPIKES = [ |
|
{ |
|
key: 'E1', label: 'E1-migration-mechanic', |
|
prompt: `${COMMON} |
|
SPIKE E1 -- migration mechanic (gates the Phase 3 plan; HIGHEST VALUE). |
|
Because all ~110 nodes share AstNode, the base storage cannot be half-slots/half-linked-list at |
|
runtime. Two candidate mechanics: |
|
(a) BRIDGE: implement slot accessors (GetChild(i)/SetChild/GetChildCount) OVER the existing linked |
|
list first -- GetChild(i) returns the i-th child in slot/schema order (map slot index -> Role, |
|
then find that role's child/children). Convert node *declarations* to partial properties |
|
family-by-family while storage stays linked-list, then a single later "flip to fields" step. |
|
(b) COORDINATED FLIP: convert storage to real backing fields in one coordinated change on a |
|
sub-branch, generator-assisted, per-family commits. |
|
Take the Expressions family as the testbed. Actually PROTOTYPE the bridge (a) for 2-3 representative |
|
nodes: BinaryOperatorExpression (two single slots), InvocationExpression (single slot + a collection |
|
slot), and IdentifierExpression (leaf). Implement the slot accessors over the linked list, add a |
|
transitional debug assert that slot order == document order, build green. Then assess what the |
|
coordinated-flip (b) would require for the same nodes (sketch, don't fully build). Report: which is |
|
more reviewable, where the bridge is awkward (childIndex over a linked list, sibling nav), whether |
|
document-order diverges from insertion order anywhere you touched, and a clear recommendation for |
|
the Phase 3 mechanic with confidence.`, |
|
}, |
|
{ |
|
key: 'E2', label: 'E2-collection-perf', |
|
prompt: `${COMMON} |
|
SPIKE E2 -- collection insert/remove renumbering perf (decides whether AstNodeCollection<T> needs a |
|
smarter index scheme). The slot model gives each node a flattened childIndex; a List-backed |
|
AstNodeCollection<T> renumbers following children's childIndex on every insert/remove -> potentially |
|
O(n^2) on large collections (a SyntaxTree's members, a big BlockStatement) under insert-heavy |
|
transforms, where the linked list was O(1). Write a standalone micro-benchmark (a small console |
|
project or a BenchmarkDotNet harness under a temp folder) comparing: (1) List-backed with |
|
renumber-on-every-mutation, (2) a linked-list baseline, across realistic sizes (e.g. 100, 1000, |
|
5000 elements) and realistic op mixes (append-heavy build, mid-insert-heavy transform, detach/move). |
|
Run it, collect timings. Then, as a sanity check on real workloads, decompile a large assembly with |
|
the existing ilspycmd (build it: ICSharpCode.ILSpyCmd) and note wall-clock (this is the linked-list |
|
baseline today). Report concrete numbers and whether renumbering is a real risk or a non-issue, with |
|
a recommendation (e.g. plain renumber vs lazy/index-on-demand vs gap-buffer).`, |
|
}, |
|
{ |
|
key: 'E3', label: 'E3-token-drop-printer', |
|
prompt: `${COMMON} |
|
SPIKE E3 -- token-drop printer probe (sizes the Phase 4 spacing/parens rework). The rewrite drops |
|
CSharpTokenNode/CSharpModifierToken child slots; their meaning moves to scalars and the printer is |
|
handed text directly. Risk: the spacing/paren machinery (InsertRequiredSpacesDecorator, |
|
InsertParenthesesVisitor, InsertMissingTokensDecorator, InsertSpecialsDecorator) may branch on token |
|
roles/presence, so dropping tokens could shift whitespace/parens. PROBE: pick ONE expression node |
|
(e.g. BinaryOperatorExpression) and ONE statement, remove their token child slots / token-role |
|
plumbing in the printer path for just those, and run the Pretty test suite |
|
(ICSharpCode.Decompiler.Tests, the Pretty/exact-text cases) to see what breaks. Inspect the diffs. |
|
Report: how many Pretty cases drift, the categories of drift (spacing vs parens vs missing |
|
punctuation), which decorators branch on token roles, and an estimate of the Phase 4 blast radius |
|
with confidence. You do NOT need the full suite green; you need the size and shape of the fallout.`, |
|
}, |
|
{ |
|
key: 'E4', label: 'E4-inheritance-shape', |
|
prompt: `${COMMON} |
|
SPIKE E4 -- inheritance shape (confirm or revisit the "flatten" decision). The design flattens: |
|
base classes declare shared child-slot members abstract (the contract), each concrete leaf |
|
re-declares its FULL slot set as partial properties in document order (override-implementing the |
|
contract), and the generator emits one flat sealed dispatch per leaf -- no base-delegation, no |
|
offset arithmetic. The alternative is base-delegation (base owns its slots, derived adds an offset). |
|
Use the TypeMembers family (EntityDeclaration contract + leaves like MethodDeclaration, |
|
PropertyDeclaration, FieldDeclaration) as the testbed. Hand-write (or generator-sketch) BOTH shapes |
|
for 2-3 of those leaves: (1) flatten -- leaf re-declares Attributes/Modifiers/Name + its own slots, |
|
flat sealed GetChild/SetChild; (2) base-delegation -- base implements its slots, leaf delegates + |
|
offsets. Compare generated-code volume (lines per leaf), ergonomics, and error-proneness (the |
|
flatten model risks a silent document-order bug if a leaf mis-orders/omits a re-declared contract |
|
slot). Report which is better for the C# AST and whether to keep the flatten decision, with |
|
confidence.`, |
|
}, |
|
] |
|
|
|
phase('Spikes') |
|
const results = await parallel(SPIKES.map(s => () => |
|
agent(s.prompt, { label: s.label, phase: 'Spikes', schema: FINDINGS, isolation: 'worktree' }) |
|
)) |
|
const findings = results.filter(Boolean) |
|
log(`${findings.length}/4 spikes returned`) |
|
|
|
phase('Synthesis') |
|
const synthesis = await agent( |
|
`You are synthesizing the Phase S de-risking spikes for the slot-based C# AST rewrite. Here are the |
|
${findings.length} spike findings as JSON: |
|
|
|
${JSON.stringify(findings, null, 2)} |
|
|
|
Produce a synthesis that resolves the open Phase-1/Phase-3 decisions the spikes were meant to settle: |
|
- Phase 3 mechanic: bridge vs coordinated-flip (from E1). |
|
- AstNodeCollection<T> renumbering scheme: plain vs smarter (from E2). |
|
- Phase 4 token-drop blast radius and any reordering of work (from E3). |
|
- Inheritance shape: keep flatten vs switch to base-delegation (from E4). |
|
For each, give the decision, the evidence that drove it, and the confidence. Then list any NEW risks |
|
the spikes surfaced and any proposed edits to ROLES_FREE_SLOT_AST_DESIGN.md (by section). Be decisive |
|
where the evidence allows; flag where a spike was inconclusive and what further probe would settle it.`, |
|
{ label: 'synthesis', phase: 'Synthesis', schema: { |
|
type: 'object', additionalProperties: false, |
|
required: ['decisions', 'newRisks', 'docEdits'], |
|
properties: { |
|
decisions: { type: 'string' }, |
|
newRisks: { type: 'string' }, |
|
docEdits: { type: 'string', description: 'proposed edits to the design doc, by section' }, |
|
inconclusive: { type: 'string', description: 'spikes that did not settle their question + the follow-up probe' }, |
|
}, |
|
} } |
|
) |
|
|
|
return { findings, synthesis }
|
|
|