Fix attribute overwrite and removal on TSyntaxNode

[partouf]

Disclosure: written with Claude Code (Claude Opus 5), which is also recorded in the commit's Co-Authored-By trailer. Everything claimed below was actually run, and the steps are reproducible from the branch.

Two bugs in attribute mutation on TSyntaxNode, both reachable from the public SetAttribute:

• Setting an attribute that is already set raises an access violation.
• Clearing an attribute leaves it in the array, so a later set returns the stale value.

Overwrite: uninitialised pointer

SetAttribute assigned its entry pointer only on the append path:

if not HasAttribute(Key) then
begin
  ...
  AttributeEntry := @FAttributes[len];
end;
if (Value = '') then RemoveAttribute(Key);
AttributeEntry^.Value := Value;   // uninitialised when the key already existed

When the key exists the if block is skipped, so the dereference below is on an uninitialised local. There is a second problem on the same line: when Value is empty it calls RemoveAttribute and then writes through the entry it just removed.

It now resolves the entry via the existing TryGetAttributeEntry whether it exists or not, and an empty value simply removes the attribute and returns.

Removal: wrong direction, no shrink, raw Move of managed strings

Index := (NativeUInt(Entry) - NativeUInt(@FAttributes[0])) + Size;
Move(Entry^, Pointer(NativeUInt(Entry)+Size)^, (High(FAttributes) * Size) - Index);
Exclude(FAttributesInUse, Key);

To delete an element the tail has to shift down onto it; this copies the doomed entry forward over its successor. The array is never shrunk, and Index mixes byte offsets with an element count. The net effect is that the key leaves FAttributesInUse while a stale entry stays in FAttributes — so a later SetAttribute appends a second entry for the same key, and TryGetAttributeEntry scanning from index 0 finds the stale one first.

Move is also the wrong tool regardless of the arithmetic: TAttributeEntry is TPair<TAttributeName, string> and moving a managed string raw corrupts its reference count. The replacement shifts by assignment and shrinks with SetLength.

Why this has not been hit

RemoveAttribute is private with SetAttribute as its only caller, and nothing in the parser sets the same key twice — so neither path is exercised today. Anything that does hits both immediately: a consumer editing a tree, or a builder recording a position it refines as it goes.

I found it writing a builder that updates one attribute per parsed directive; the second update crashed.

Verification

AST.SetAttributeTwice covers write, overwrite, clear, and re-set. It is a plain node — no parsing involved:

state	result
before this change	EAccessViolation, write to an invalid address, on the overwrite
with only the SetAttribute half	fails on the re-set: expected "again", got "second"
with both	passes

Full suite on Delphi 13 Win32: 42 passed of 43. The one failure is pre-existing and unrelated — Serialization.BinaryRoundTrip, where line_seq holds a pointer value that does not survive a round trip. FPC via this repo's own workflow: all green.