fix(incremental): propagate cache staleness to dependents of edited units
When a unit's source content changed, it was correctly recompiled from a warm --unit-cache, but a cached unit that USES it kept its cached interface and was imported in the cached-iface phase — which runs BEFORE source-recompiled units are analysed. Importing the cached dependent then failed to resolve a type defined only by the not-yet-analysed dependency, raising EImportError (empty message), e.g. editing blaise.codegen.driver and rebuilding the whole compiler warm broke on blaise.frontend.opts (TBackendKind unresolved). The same corrupted warm cache also produced an intermittent SIGSEGV in the e2e runner. uUnitLoader now tracks units taken via the source path (FSourceLoadedNames). In the cached path, after recursing into a unit's interface-use dependencies, if any dependency was source-loaded the cached iface is discarded and the unit is recompiled from source too, so it is analysed after its dependency. Post-order DFS makes this transitive. fixpoint-warmcache.sh now edits a dependency unit's content between its two builds (previously two clean fills that never exercised staleness propagation); it FAILS on the pre-fix compiler and PASSES on the fixed one.
This commit is contained in:
Graeme Geldenhuys
parent
48569f36d1
commit
3ff19c0a00
|
|
@ -62,6 +62,11 @@ type
|
|||
FDefines: TStringList; { not owned — conditional symbols for each unit's lexer }
|
||||
FLoading: TStringList; { units currently on the load stack — cycle detection }
|
||||
FLoadedNames: TStringList; { units already fully loaded }
|
||||
FSourceLoadedNames: TStringList; { units taken via the SOURCE-recompile
|
||||
path (stale cache or no cache). A
|
||||
cached unit whose dependency is in
|
||||
this set must itself be recompiled —
|
||||
staleness propagates up the uses graph. }
|
||||
FResult: TObjectList; { the in-progress output list (not owned here) }
|
||||
FPrebuiltIfaces: TObjectList; { owned TUnitInterface }
|
||||
FPrebuiltObjectPaths: TStringList;
|
||||
|
|
@ -79,6 +84,9 @@ type
|
|||
reconstitute a TUnitInterface. Returns nil on failure. }
|
||||
function LoadIfaceFromObject(const APath: string): TUnitInterface;
|
||||
function LoadOne(const APath: string): TUnit;
|
||||
{ True if any interface-use dependency of AIface was taken via the
|
||||
source-recompile path (is in FSourceLoadedNames). }
|
||||
function DependsOnSourceLoaded(AIface: TUnitInterface): Boolean;
|
||||
procedure LoadTransitive(const AName: string);
|
||||
{ Collect the .o for an impl-only dependency (and its transitive
|
||||
dependencies) for linking, without parsing source or importing its
|
||||
|
|
@ -288,6 +296,17 @@ begin
|
|||
end;
|
||||
end;
|
||||
|
||||
function TUnitLoader.DependsOnSourceLoaded(AIface: TUnitInterface): Boolean;
|
||||
var
|
||||
I: Integer;
|
||||
begin
|
||||
Result := True;
|
||||
for I := 0 to AIface.UsedUnits.Count - 1 do
|
||||
if FSourceLoadedNames.IndexOf(AIface.UsedUnits.Strings[I]) >= 0 then
|
||||
Exit;
|
||||
Result := False;
|
||||
end;
|
||||
|
||||
procedure TUnitLoader.LoadTransitive(const AName: string);
|
||||
var
|
||||
Path: string;
|
||||
|
|
@ -332,24 +351,43 @@ begin
|
|||
try
|
||||
for I := 0 to Iface.UsedUnits.Count - 1 do
|
||||
LoadTransitive(Iface.UsedUnits.Strings[I]);
|
||||
FPrebuiltIfaces.Add(Iface);
|
||||
FPrebuiltObjectPaths.Add(ObjPath);
|
||||
FLoadedNames.Add(AName);
|
||||
{ Impl-only dependencies are not reachable via the interface uses,
|
||||
but their objects must still be linked or the program loses their
|
||||
code (an incremental rebuild that loads this unit from its cached
|
||||
.bif would otherwise drop them). They are collected for LINK ONLY
|
||||
— not semantically imported — because the consuming unit never
|
||||
references their symbols, and importing their ifaces early would
|
||||
break the dependency-ordered import (impl/interface-use cycles, e.g.
|
||||
a backend unit whose interface a peer impl-uses, cannot be resolved
|
||||
by the leaf-first iface import). }
|
||||
for I := 0 to Iface.ImplUsedUnits.Count - 1 do
|
||||
CollectLinkOnlyObject(Iface.ImplUsedUnits.Strings[I]);
|
||||
{ Staleness propagation: if any interface-use dependency was taken via
|
||||
the SOURCE path (its cache was stale, or it has no cache), then THIS
|
||||
unit's cached iface may reference types whose definitions are only
|
||||
available from that recompiled dependency — and those are imported in
|
||||
a later phase than cached ifaces, so resolution would fail with an
|
||||
EImportError ("field type X unresolved"). Discard the cached iface
|
||||
and recompile this unit from source too, so it lands in the
|
||||
source-ordered analysis phase after its dependency. (Without source
|
||||
on the path we cannot recompile, so the cache is the only option —
|
||||
keep it when source is not locatable.) }
|
||||
if DependsOnSourceLoaded(Iface) and (Locate(AName) <> '') then
|
||||
begin
|
||||
Iface.Free();
|
||||
Iface := nil; { fall through to the source-compile path below }
|
||||
end
|
||||
else
|
||||
begin
|
||||
FPrebuiltIfaces.Add(Iface);
|
||||
FPrebuiltObjectPaths.Add(ObjPath);
|
||||
FLoadedNames.Add(AName);
|
||||
{ Impl-only dependencies are not reachable via the interface uses,
|
||||
but their objects must still be linked or the program loses their
|
||||
code (an incremental rebuild that loads this unit from its cached
|
||||
.bif would otherwise drop them). They are collected for LINK ONLY
|
||||
— not semantically imported — because the consuming unit never
|
||||
references their symbols, and importing their ifaces early would
|
||||
break the dependency-ordered import (impl/interface-use cycles, e.g.
|
||||
a backend unit whose interface a peer impl-uses, cannot be resolved
|
||||
by the leaf-first iface import). }
|
||||
for I := 0 to Iface.ImplUsedUnits.Count - 1 do
|
||||
CollectLinkOnlyObject(Iface.ImplUsedUnits.Strings[I]);
|
||||
end;
|
||||
finally
|
||||
FLoading.Delete(FLoading.IndexOf(AName));
|
||||
end;
|
||||
Exit;
|
||||
if Iface <> nil then
|
||||
Exit; { cached path taken — done. Otherwise fall through to source. }
|
||||
end;
|
||||
end;
|
||||
|
||||
|
|
@ -373,6 +411,11 @@ begin
|
|||
{ Append this unit after all its dependencies }
|
||||
FResult.Add(U);
|
||||
FLoadedNames.Add(AName);
|
||||
{ Record that this unit was recompiled from source, so any cached unit
|
||||
that depends on it (interface-use) propagates the staleness and also
|
||||
recompiles — see the staleness-propagation guard in the cached path. }
|
||||
if FSourceLoadedNames.IndexOf(AName) < 0 then
|
||||
FSourceLoadedNames.Add(AName);
|
||||
U := nil; { ownership transferred to FResult }
|
||||
finally
|
||||
U.Free(); { no-op if U = nil (success path) or on error }
|
||||
|
|
@ -390,6 +433,8 @@ begin
|
|||
FLoading.CaseSensitive := False;
|
||||
FLoadedNames := TStringList.Create();
|
||||
FLoadedNames.CaseSensitive := False;
|
||||
FSourceLoadedNames := TStringList.Create();
|
||||
FSourceLoadedNames.CaseSensitive := False;
|
||||
FPrebuiltIfaces := TObjectList.Create(True);
|
||||
FPrebuiltObjectPaths := TStringList.Create();
|
||||
FPrebuiltObjectPaths.CaseSensitive := False;
|
||||
|
|
@ -405,6 +450,7 @@ begin
|
|||
FLinkOnlyObjects.Free();
|
||||
FPrebuiltObjectPaths.Free();
|
||||
FPrebuiltIfaces.Free();
|
||||
FSourceLoadedNames.Free();
|
||||
FLoadedNames.Free();
|
||||
FLoading.Free();
|
||||
inherited Destroy();
|
||||
|
|
|
|||
|
|
@ -91,18 +91,39 @@ fi
|
|||
SIZE1=$(stat -c %s "$STAGE2A")
|
||||
echo " build1 ok — stage-2a size = $SIZE1 bytes"
|
||||
|
||||
echo "[2/4] BUILD 2 — recompile into the SAME (now populated) unit-cache"
|
||||
# This is the path under test: dependency units load from cached .bif/.o and
|
||||
# the top program is recompiled against those cached interfaces.
|
||||
# EDIT a DEPENDENCY unit between the two builds. This is what a real
|
||||
# incremental rebuild looks like (edit one unit, rebuild) and is the only thing
|
||||
# that exercises STALENESS PROPAGATION: a unit whose cache is now stale must be
|
||||
# recompiled from source, AND every cached unit that depends on it must also be
|
||||
# recompiled — otherwise the cached dependent's iface is imported before the
|
||||
# recompiled dependency's types exist, and resolution fails with EImportError.
|
||||
# blaise.codegen.driver is depended on by blaise.frontend.opts (TBackendKind),
|
||||
# so editing it triggers exactly that propagation. A harmless trailing comment
|
||||
# changes the content hash without changing behaviour. Restored on exit.
|
||||
EDIT_UNIT="compiler/src/main/pascal/blaise.codegen.driver.pas"
|
||||
cp "$EDIT_UNIT" "$WORK/edit_unit.bak"
|
||||
restore_edit() { cp "$WORK/edit_unit.bak" "$EDIT_UNIT" 2>/dev/null || true; rm -rf "$WORK"; }
|
||||
trap restore_edit EXIT
|
||||
printf '\n{ warmcache-fixpoint content-edit probe }\n' >> "$EDIT_UNIT"
|
||||
|
||||
echo "[2/4] BUILD 2 — edit a dependency unit, then recompile into the SAME cache"
|
||||
# Path under test: the edited unit is stale (recompiled from source); its
|
||||
# cached dependents must propagate that and recompile too, and unedited
|
||||
# dependency units still load from cached .bif/.o.
|
||||
set +e
|
||||
"$STAGE1" --source compiler/src/main/pascal/Blaise.pas \
|
||||
--output "$STAGE2B" --unit-cache "$UC" $UNIT_PATHS \
|
||||
> "$WORK/build2.log" 2>&1
|
||||
B2_RC=$?
|
||||
set -e
|
||||
# Restore the edited unit immediately — its content is no longer needed.
|
||||
cp "$WORK/edit_unit.bak" "$EDIT_UNIT"
|
||||
if [ "$B2_RC" -ne 0 ]; then
|
||||
echo "BUILD2_FAIL (warm-cache rebuild, exit=$B2_RC)"
|
||||
echo " (a unit loaded from the cache likely produced a broken round-trip)"
|
||||
echo "BUILD2_FAIL (warm-cache rebuild after content edit, exit=$B2_RC)"
|
||||
echo " (likely a stale-cache propagation bug — a cached dependent of the"
|
||||
echo " edited unit was imported before the recompiled unit's types existed,"
|
||||
echo " raising EImportError; see grep 'recompiling from source' below)"
|
||||
grep -iE "stale|EImportError|recompil" "$WORK/build2.log" | head
|
||||
tail -15 "$WORK/build2.log"
|
||||
exit 1
|
||||
fi
|
||||
|
|
|
|||
Loading…
Reference in a new issue