-
Notifications
You must be signed in to change notification settings - Fork 423
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dyno: initial implementation of checking (explicit) interface constraints #26396
Open
DanilaFe
wants to merge
72
commits into
chapel-lang:main
Choose a base branch
from
DanilaFe:dyno-interfaces-new
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
This is a shortcut for resolving the call `I(t1, t2, ...)`. Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Since type queries don't currently work while resolving the expressions, there's no reason to pretend they do in the code. Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
0 = implements interface, 2 = no interface found Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR lays the foundation for interface support in Dyno; specifically, it makes it possible to verify that a type implements an interface via an
implements
statement or via a definition likerecord foo : someInterface
. The goal here is to implement the functionality required by stabilized standard library features. Specifically, it's desirable to support the outcomes of the special method naming discussion by supportinghashable
,serializable
, etc., theimplement interface
primitive, as well as theSort
module discussion in the form of thesortComparator
interface.The Predicate interpretation of interfaces
We can think of interfaces as predicates on types; an interface definition as follows:
Defines a predicate
I(t)
which is satisfied when a methodt.foo()
is present. This generalizes well to multi-argument interfaces, even though they are not officially stable: an interfaceDefines a binary predicate
I(T1, T2)
that holds when one of the two types can be cast to another.InterfaceType
encodes the predicateWhen we are searching for a witness to prove a predicate, we want to state what it is we are trying to "prove". As a result, it's convenient to have an encoding for a claim like
I(t1, ..., tn)
, even if we haven't found a proof for it yet, or never will. I elected to make this object -- which just contains the interface ID and a mapping of its formals to requested types -- itself a type. I think this makes sense because interface predicates can occur in expressions and statements:As a result, it's easier to just treat the interface constraint as another sort of type that can be constructed. This PR takes that route, defining a new type constructor for interfaces (
default-functions.cpp
, commits aba8332, e157006, cfa3b26). From there, it simply relies on the standard resolution process.Implementation points
There are several ways to implement an interface:
Generally speaking, all of these map to a predicate application
I(myRec)
. Thus, it became convenient to treat them in an (almost) unified manner, instead of resorting to constant AST-based pattern matching. To this end, I introducedImplementationPoint
, which contains the interface ID, the ID of the AST node that causes the implementation, and a list of actuals / arguments to the predicate. Unfortunately I had to treat themyRec : I
case specially because resolving the type ofmyRec
involves resolving the aggregate declaration rather than searching for an expression's type.To check whether an interface predicate (in the form of an
InterfaceType
) is satisfied, one needs to find all implementation points available, find the one that applies to the given interface and types, and check whether at that point, the requirements for the predicate (e.g., the presence ofSelf.foo()
) are met.In implementing the implementation point search, I observed that in production, visibility of implementation points is propagated only via unrestricted
use
statements. Thus,import M.{R}
would not bring in theimplements
facts forR
, even if they are in the type's declaration. In general, it's not possible to "name" a standaloneimplements
statement, which means it's impossible to mention it in a restriction on something likeimport
; as a result, I chose to always bring inimplements I(T)
statements into a scope through mentions of modules in visibility statements. Leaning on this approach, it was easiest to also always bring in implementation points from type declarations likeR : I
. This is perhaps inefficient (we see more implementation points than might be usable), but it's sound (if a type isn't brought in, its implementation point is irrelevant) and perhaps even necessary if a type isn't directly visible, but is, e.g., contained via another imported type:Finding matching implementation points
To support the standard library today, we need support for generic implementation points, such as
implements hashable(_tuple)
. In general, there may not be only a single matching implementation point for any application of an interface predicate.At first, I wanted to treat finding a matching implementation point as a function call, and piggy-back on the disambiguation process to select the most specific generic implementation point if need be. I still think this is desirable, but there were a number of challenges and observations in the way:
proc
that corresponds to the implementation point. This is fine by itself, but the code that mapsIDs
to their (generated) ASTs is relatively brittle (seebuilderResultForDefaultFunction
) and relies on name-based comparisons. It might be possible to get it to work with implementation points, but it did not seem worth it for a first prototype.instantiateSignature
to handle the case of an interface implementation function.instantiateSignature
is already very complex and hard to maintain, and additional tweaks will only make it more so.type
formals, so conversions, coercions, and other measures of specificity are less important.As a consequence of the above bullets, this PR matches the behavior of production, by simply iterating the list of visible implementation points, selecting the first concrete point if one exists, and otherwise selecting the first generic point.
Checking Interface Constraints
Once an implementation point is found, the last step is to check if the interface's requirements (methods, associated types) are met. When it comes to "subtyping" or compatibility, functions are contravariant in their inputs, and covariant in their outputs. Thus, a function that satisfies an interface must have formals that are supertypes of the interface function's signature, and a return type that is a subtype of the interface function's return type. Thus, when searching for a function
proc foo(arg1: A, ... argN: Z)
, set up a call forfoo(A, ..., Z)
and resolve it. This finds a function that accepts the required types, and possibly more (i.e., the formals are supertypes). From there, it's necessary to check if the return type of the function can be passed to the type expected by the interface (in other words, if it's a "subtype").It was suggested in the team chat that we ought to treat formal names as part of the iterator's interface. As a result, this implementation feeds all arguments by name
foo(arg1 = A, ..., argN = Z)
, and then ensures that this does not cause re-ordering of formals -- that way, both by-name and nameless calls ought to work.The trickiest aspect is that some of our standard interfaces (particularly serializers and the
keyComparator
interface) require generic functions, sometimes with type queries. Here's one offender (not defined in production because prod. can't handle it):Here, the type query constraints are important, and the ability to accept any serializable type is important. Thus, when checking interface constraints, what we want is the ability to "match up" type queries. This PR does this by introducing opaque --- but concrete --- placeholder types for type queries in the interface's function. The above signature becomes:
Because they are unique, these types cannot be passed to concrete formals (i.e.,
ref serializer: binarySerialier
will not accept aPlaceholder(ST)
, because the latter is only compatible with itself). This ensures that the candidate function is generic in that spot, or references a previously queried type parameter. To make this work, the PR adds a new flag (usePlaceholders
) inResolver
that configures it to create placeholder types in place ofAnyType
.Placeholders are also used for interface formals (e.g.,
Self
) when getting the signature of a formal's required function. This is because an interface is intended to encapsulate all the required properties of a type. IfSelf
is immediately instantiated with a type, then in cases likeproc Self.foo()
, this type can be used as the implicit receiver for calls, which introduces "hidden" dependencies on methods that aren't clear from the interface declaration. Using an opaque placeholder type likePlaceholder(Self)
ensures that no "external" methods slip in, and that the signatures in the interface are self-contained.Thus, the typed signatures of the interface's required functions have placeholders for
Self
; however, when searching for candidates that satisfy these signatures, we need to use the actual type ofSelf
(e.g,int
inI(int)
). Thus, this PR adds a "substitute" method onType
and related types to replace placeholders with their actual values. Note that this does not replace the placeholders from generic type queries.TODO before this PR is non-draft
error immediately for concrete implementation pointscomments to new headersnicer stringify function for InterfaceTypemove interface types into a new headerimplementation point formals should be called actualsuseconst Type*
for substitution instead ofQualifiedType
Future work
foo(x : I)