Use RAII to clean up temporary graph names

[soutaro]

Temporary graph names now live in an RAII guard that restores their name and string reference counts when dropped. This removes manual cleanup bookkeeping from constant resolution and completion.

The previous nesting_stack_to_name_id API returned a separate list of names for callers to untrack; the scoped result now owns that cleanup.

Although this changes the Rust APIs in name_api, they are not exposed through the C FFI, so the C API and ABI remain unchanged.

[vinistock]

This is a great idea and much more robust code than what we currently have, but I want to propose we actually go one step further.

The fact that we need a mutable graph to add names and then untrack is actually technical debt we never addressed. It is required that consumers call graph.resolve before trying to invoke something like graph.resolve_constant. After resolution runs, every name that exists in the program is guaranteed to have already been added to the graph.

Instead of making this improvement, can we please try to avoid having to add names and use a mutable graph altogether? I think we can get there with these steps:

• We still need to build the Name structs. We just won't add them to the graph
• Instead of calling resolution, which is the part that requires mutability, we know already that the consumer has to have invoked graph.resolve, so if the name they are trying to resolve truly exists in the graph and has successfully resolved, then we will already know about it. We can basically do this:

fn resolve_constant(...) {
  let name_id = name_api::nesting_stack_to_name_id(&const_name, nesting);

  // Not mutable!
  with_graph(pointer, |graph| {
    match graph.names.get(&name_id) {
      Some(NameRef::Resolved(name)) => {
        // return the declaration
      }
      Some(NameRef::Unresolved(_)) |
      None => ptr::null()
    }
  })
}

[soutaro]

@vinistock I don’t think a simple lookup of registered names will work. The motivating use case is resolving class names in inline RBS comments, so those references may not yet exist in the graph. (LSP completion has a similar requirement when checking whether a shorter constant name resolves to the expected declaration.)

An immutable version of resolve_constant would make sense, though. Should we try implementing that?

[vinistock]

I think we should treat constant references in RBS comments as a proper constant reference. Otherwise, we wouldn't be able to find them as usages, we wouldn't be able to rename declarations automatically and so on.

We should be able to treat resolve_constant as a simple lookup after resolve. Any constant that appears in the code has to be modelled as a constant or else we get missing data in other parts of the analysis.

[soutaro]

We can treat type name references in RBS comments as proper constant references. 👍 (But when will we do it?)

However, this doesn't help completions to work:

1. User picks a typename/constant from a completion list
2. The server tries to resolve the fully qualified constant name into the basename, if the basename can be resolved to the full name

The second step requires resolving unresolved constant name.

[vinistock]

But when will we do it?

As soon as we start properly indexing RBS sigs. There are no requirements, so we can start right away.

However, this doesn't help completions to work

Completion doesn't use the resolve constant API at all. We have a dedicated API for completion candidates which provides consumers with all reachable declarations based on the surrounding context.

Consumers receive all possible candidates for the context and then, if desired, filter them based on the incomplete name being typed.

[vinistock]

Chatted with @soutaro. We agreed that we should allow consumers of the API to request the resolution of a constant that isn't necessarily found in the source code (i.e.: would this reference resolve in this namespace?).

So we're keeping the mutable API that actually runs the algorithm.