Dyno: move to a more principled resolution strategy

[DanilaFe]

Dyno, by its nature as a library, has been written to compute "as much as possible" when performing resolution. Even if one variable in a module is incorrectly declared (emitting an error), Dyno can proceed to resolve other variables in the module. Since Dyno doesn't immediately halt upon errors, to prevent cascading failures, Dyno's resolver will skip resolving calls when they have erroneous expressions as arguments, or whose arguments depend on erroneous expressions.

Initial Signatures

This skipping logic has been re-used for a second mechanism in Dyno: the 'initial function signature'. This is a kind of optimization. In the presence of generics, each call can in theory produce a different instantiation of a generic function. It would be costly to recompute instantiated functions for each call if we can tell "at a glance" that they will fail. For example:

proc foo(x, y: f(x.type), z: g(y.type), int) {}
foo(x, y, z, "hello");

Above, even though generic resolution dictates we need to go through each formal at a time to compute successive substitutions for x, y, and z, the call is "destined to fail" since the last argument is a string, but the last formal is an integer. To short-circuit these cases, Dyno computes a single "initial signature" for any function, which doesn't external type information, and is by nature partial. In the case above, it will be roughly:

proc foo_initial(x: Unknown, y: Unknown, z: Unknown, int) {}
foo(x, y, z, "hello");

Unknown types are permissively permitted to accept any actual. Checking initial signatures can be done quickly, without computing substitutions, just by testing canPass on corresponding formals.

To compute initial signatures in the presence of necessarily missing information (e.g., substitutions from the call), Dyno's skipping logic has been written to avoid resolving some calls. The precise definition of the skipping logic (captured in shouldSkipCallResolution) has evolved over time; however, it roughly boiled down to skipping calls with generic actuals with some exceptions. These exceptions, however, grew to be very complicated.

Problems with shouldSkipCallResolution

Call calls, whether it be during initial signature computation or "regular" resolution, go through the skipping logic, at the very least because we still need to skip erroneous computations. However, this led to a tension between the two contexts in which the skipping logic is used. When computing initial signatures, we tend to err on the side of skipping. The worst that can happen when skipping a call is that we produce an UnknownType, which allows the initial signature to be considered for "real" instantiation, and discarded there if necessary. However, when computing "final" signatures, or resolving elsewhere, we don't want to skip when we can help it: "real" functions can sometimes accept generic arguments (when they are types and params), and to match production, we eventually need to resolve them.

This led to a list of workarounds in shouldSkipCallResolution which try to strike the right balance between skipping and not skipping, sometimes trying to divine nearby clues whether we are computing an initial or a final signature. PRs as recent as #27963 introduced new workarounds and heuristics.

However, the bottom line is that in general, identical calls ought to be skipped in some contexts, and resolved in others. Consider this example:

proc foo(type x: numeric) param { if isGenericType(x) then compilerError("I don't support generic types!"); return true; }
proc bar(type x: numeric) where foo(x) {}
bar(int); // fine
bar(numeric); // not fine

The function foo doesn't support generic types. Thus, foo(numeric) will produce a compiler error. This is a distillation of the chpl__buildArrayRuntime type, which we've recently run into problems with. Notice that during the initial signature resolution of bar, before the arguments are incorporated, the foo(x) call is foo(numeric). However, we shouldn't emit an error yet, since we are simply missing some information. However, during bar(numeric), the same call foo(numeric) in the where clause ought not to be skipped, and should be resolved and trigger the error. Thus, how we treat foo(numeric) is context-sensitive.

The new approach

To resolve the tension, I've decided to simply distinguish, in the shouldSkipCallResolution logic, between calls in "initial" vs. "real" signatures. Outside of "initial" signatures (and, also, partial instantiations of types, which similarly work with partial information), this PR only skips call resolution for errors (the original motivating case).

Since we can now make this distinction, this approach removes a lot of special cases in shouldSkipCallResolution that served to "uphold the balance". It also removes shouldUseUnknownTypeForGeneric, which was an earlier form of what is now done by the shouldSkipCallResolution mechanism.

The PR adjusts the logic of shouldSkipCallResolution by investigating the question: when do we want to skip calls?

1. Coarsely, we might want to skip all calls with generic arguments, since they may be generic due to missing instantiations. We could skip foo(x) in example above, because x is numeric.
2. However, the simple rule disables a lot of useful cases. For instance, class? (which is a call ?(class)), is skipped, because class is generic. Intuitively this call shouldn't be skipped, since class is well-defined and "static". On the other hand, 'x' may well be "dynamic", since it can receive a substitution from the call site.
3. This leads to the general direction of the new approach: generic arguments and types are skipped if they could be swayed by substitutions. Thus, verbatim references to generic types like numeric or class are not skipped. This enables patterns like class?, owned myGenericType, and more. But what actuals/arguements can be affected by substitutions?
   • For identifiers and dot expressions, the answer is clear from context: if they refer to a formal/field that may yet be substituted, but hasn't been yet.
   • For complex expressions, it's if any of the sub-expressions can be affected by substitutions. A proxy for this, assuming the sub-expressions themselves go through the same skipping logic, is whether they have been skipped (and are marked Unknown).
   • ...the above works, but disqualifies expressions like (t1, t2) for generic t1, t2. Tough this can make sense (in general, type constructors do not expect to be instantiated with incomplete information, and (t1, t2) is a type constructor), the tuple type constructor is particularly well-behaved, and we can do more. So, as a special case, allow tuples to be resolved with partial arguments. This is intended strictly as an optimization, so that we can reject initial signatures earlier. It's not intended to affect resolution semantics.
     • However, this means the proxy for substitution-sensitive complex expressions is no longer accurate, since we might have a non-skipped tuple call that was built from unsubstituted formals, so we track it explicitly.
     • For this, introduce a new isPartialResult field on ResolvedExpression, which corresponds to whether we skiped call resolution, OR would've skipped it if not making the semantic-preserving tuple improvement. This way, though (t1, t2) is resolved, calls like doesntExpectPartial((t1, t2)) are not until t1 and t2 are substituted.

I've applied this approach to both generic params and types. All existing Dyno tests pass (which lock down previous workarounds' goals), as does the previously-failing test types/records/bharshbarger/coercedImmediate.

...While there

It turns out that we were skipping some calls that ought not to be skipped (yay for the new approach!). To fix up testing, I did or will do the following:

• WONTFIX (this PR is huge) enable casts to owned. These are compiler-generated. Seems very similar to Dyno: implement builtin borrow #27152.
• Fix bug in generic multi-variable declarations. It turns out our code wasn't handling var x, y, z because it expected to find at least one declaration with a type or initialization expression. When it doesn't, instead of assigning x, y and z the AnyType as it should in the non-multi-decl case, it instead left them unknown.
• Fix bugs in handling of ? which is trailed by other type parameters. The logic didn't account for differing indexing schemes between uast::FnCall and resolution::FormalActualMap. The former assigns indices to ?, but the latter does not. Also, CallInfo::createUnknown didn't correctly computed by-name actuals (for a similar indexing reason).
• Match production in disallowing variables with generic types from being default-initialized in generated initializers, even if they have a fixed type given substitutions. @benharsh mentions that this is because there's no way for a user to write an explicit initializer with the same behavior.
• Match production in making compiler-generated initializers for records treat formals with array type expressions as default rectangular arrays (normally, array type expressions are management-generic)
• Fix emitting the same error multiple times if the erroneous expression is in a generic function's formal expression. This was a problem on main already:

  proc idk(param x) type : int {
   compilerError("nooo");
    return int;
  }
  record wrapper { type t; }
  proc makeWrapper(type t) type do return wrapper(t);
  proc foo(param p, arg: makeWrapper(idk(p))) {}
  foo(1, new wrapper(int));

  However, it became a problem after my initial changes because, as mentioned before, skipped calls (which now appeared more reliably, avoiding other errors) create "Unknown" types, which accept any actual.
  • The change to this is sweeping. In particular, I am now configuring calls to fail to resolve if any error was emitted during their resolution, even if a known type was determined for a formal. I think this is important for semantic consistency. There are two parts to this. First, for any given candidate, if an error occurred during resolving its formal, even if we computed a final type, the formal should not accept the actual. This way, if we observe that an initial signature produces errors, we avoid going forward with resolving the instantiated signature, which may re-emit the error (and in any case, an error message indicates something went wrong). Second, just rejecting a single candidate in this way is a bad idea because it leads to something like SFINAE in c++: a more specific candidate with an erroneous formal may be rejected, and a fallback, less-specific (but non-errorring) candidate would be picked instead. Thus, we may unintentionally pick and resolve a potentially arbitrarily complex fallback candidate that wouldn't even be picked under normal circumstances.
  • To implement the above in a fashion that works under the query system, I initially tried to hook into the runAndDetectErrors functionality of the query system. However, this is too broad: to be precise, runAndDetectErrors tracks if errors were occurred while computing any required information for the called queries. In particular, this includes unrelated syntax resulting from parsing a module in which a function / variable / etc were declared. Though sound, discarding candidates for this reason goes against our attempts to provide as much information as possible in the editor.
  • runAndDetectErrors functionality previously always hid error messages emitted while detecting errors, the use case being __primitive("resolves", ...) and friends. As I was using it, however, I needed to show errors to the user as before. Thus, I had to adjust the logic to cope with potentially having issued errors. Even though I didn't end up using it, I kept it in case it ever comes up again.
  • Leaning on this logic uncovered a bug in the query system, in which queries that errored once would forever be marked as errored, even in later generations. The solution was to reset that flag in canUseSavedResultPushIfNot.
  • Finally, instead of runAndDetectErrors, I settled on only counting "local" errors for the purposes of candidate rejection. A proxy for a "local" / relevant error is whether or not it was issued directly by the Resolver that's handling the candidates' formal. To track this, I added a new field in the resolver, and adjusted all calls to error and report to set that field.

[benharsh]

Awesome! Just a couple of questions.

[benharsh]

Can silenceErrors() be different at different levels of the stack?

[DanilaFe]

Now that I think about it, although it can, looking at the top of the stack is not right. This field ought to track whether the user saw errors from this field. They didn't if any of the stack entries was silencing errors.

[benharsh]

Can we get rid of this now?

[benharsh]

I wonder if we should change this (in a separate effort). I'm guessing this would have been motivated by the use of ? in type constructors, rather than function calls.

[benharsh]

Very nice!