Skip to content

Symbol.WriteFlag is an optional slot field on a value type, propagated by hand #69

Description

@thiremani

Summary

Symbol.WriteFlag (added in 2f9765c) records the i1* that says whether a logical
output was actually written. The capability is required — it is how a skipped callee
write preserves the caller's value — but Symbol is a poor home for it.

Symbol is already a union of three things: a value, a storage slot, and a
parameter. WriteFlag is only meaningful for the slot flavor. The code says so:

// A write flag belongs to the storage slot, not to an ordinary value read.
newS.WriteFlag = llvm.Value{}

derefIfPointer has to explicitly erase the field, which is the tell that it does
not belong on the type.

Why it matters

Because the field rides on a shared struct, it has to be propagated by hand:

  • GetCopy must remember to copy it
  • cond.go re-threads it at four separate sites (220, 240, 356, 368)
  • makePtr and promoteAlias each re-attach it

and markOutputSlotWritten compares it by identity:

if !mergedFrom.IsNil() && mergedFrom == dst.WriteFlag {
    return
}

So correctness depends on the same flag pointer being threaded through every
hand-off. Drop one and a merge silently becomes a fresh write — no type error, no
verifier complaint, no test failure. This is the same class of Symbol-aliasing
fragility that produced the comparison-LHS leak fixed in c79a918, where marking a
shared *Symbol mutated the scope's own binding.

Suggested direction

Give staged outputs their own type rather than widening Symbol, e.g.

type outputSlot struct {
    ptr     *Symbol    // always Ptr-typed storage
    written llvm.Value // i1*, never nil
}

Then:

  • values cannot carry a meaningless write flag, and derefIfPointer needs no erase
  • GetCopy has nothing extra to remember
  • the four cond.go hand-offs become structural rather than manual
  • markOutputSlotWritten's identity comparison operates on a type that is always a slot

Scope

Not a bug report — no known miscompile today. This is a structural follow-up
deliberately kept out of PR #65, which is already large and touches the ownership
core. Worth doing before more code accretes around the current shape.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions