Commit 7268f636 authored by Matthew Fernandez's avatar Matthew Fernandez
Browse files

move state offset calculation into symbol resolution instead of reindexing

The offset calculation did not have much to do with reindexing and I think it
may have just been put here because it was a convenient post-resolution place
for it. An unfortunate side effect of this design was that resolved referents
had an invalid offset. To see why this is, note that symbol resolution (1)
replaces null referents with a *copy* of the target node and (2) runs prior to
reindexing. That is, the offset calculation in reindexing would only affect the
original state VarDecl, not its copies in ExprID referents.

None of what has just been described was a bug. The offsets of the VarDecl
copies are never used. But its presence incorrectly suggests to librumur clients
that it is usable.

In this commit we move the offset calculation to its more natural place within
symbol resolution. We do offset calculation *prior* to declaring a VarDecl in
the symbol table. This results in ExprIDs now receiving a referent with correct
offset information. Note that we need to do some extra checks because a
just-parsed VarDecl is part of an unvalidated model and may be invalid.

All this is part of a broader direction to make all the fields of a referent
valid and usable.
parent 45f095c9
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment