Skip to content

Temporaries consumed inline are never freed — including the bare auto-propagating call the compiler recommends #57

Description

@djabi

A temporary produced by a call or an index and consumed inline is never freed. Binding it to a local first is always clean.

Split out of #39 (T1711) at George's prompt: that issue is a temporary freed twice and aborting, this is a temporary never freed and staying silent. Same family, opposite failure, and almost certainly a different path — filing separately so fixing one does not quietly close the other.

Three shapes, all minimal

1. A bare auto-propagating call. This is the one that matters, because the compiler recommends it.

_make!(string move s) string {
  if s.is_empty { raise error(message: "empty"); }
  return s + "!";
}

_bare!() int {
  string[] out = string[]();
  string x = "a";
  out.push(_make(move x));      // leak: 1 allocation not freed
  return out.len;
}

_explicit!() int {
  string[] out = string[]();
  string x = "a";
  out.push(_make(move x)?^);    // clean
  return out.len;
}

Same value, same call, same consumer. The only difference is the ?^.

The diagnostic for a misplaced ? puts the leaking form first and frames it as idiomatic:

bare `?` is not an error-handling operator in Promise
  - drop the `?` — inside a failable (`!`) function a bare call already auto-propagates:  foo()
  - to propagate explicitly:          foo()?^

2. A map index unwrapped into a call argument.

_take(json.JsonValue v) bool { ... }

assert(_take(o["b"]!), ...);      // leak: 1 allocation not freed

json.JsonValue b = o["b"]!;       // clean
assert(_take(b), ...);

3. A method call on an array index.

C? got = xs[0].t();               // leak: 3 allocations not freed

S one = xs[0];                    // clean
C? got = one.t();

An absent optional in shape 3 still leaks 1, so it is not about what the value holds.

What it cost in practice

Writing a JSON decoder for the gate manifest in base: a helper with two bare calls in a loop leaked 300 allocations parsing one realistic document. Adding ?^ to those two lines took it to 8. Nothing looked wrong, and the error messages had recommended exactly what I wrote.

The workaround throughout is now "bind every temporary to a local before consuming it," which makes the code substantially more verbose than it should be. gate/decode.pr carries a comment saying why.

One case I could not isolate

After fixing every bare call I could find, that decoder still leaks 8 allocations on a realistic manifest and 1 on a small one. I narrowed it to specific fields — a host_arch array leaks where an identically generated host_os array does not; a present optional fix leaks where an absent one does not — but extracting those shapes into standalone tests passes clean every time. So there is at least one more case here that only appears at some larger composition, and I would not assume the three above are the whole set.

Reproducible tree: promise-language/base, gate/decode.pr with gate/decode_test.pr — 35 tests pass, 4 leak.

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

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions