-
Notifications
You must be signed in to change notification settings - Fork 122
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
Ad hoc polymorphism #703
Ad hoc polymorphism #703
Conversation
We make the lazy constraint mechanism more powerful by having the solver attempt to expand a lazy constraint every time it learns something new about the type of its trigger expression, not just when the type becomes fully concretized. For example, a constraint that checks that a variable is of a container type can be expanded as soon as the top-level definition has been resolved (i.e., we know whether it is a set or a vector) even if the element type of the container is not yet known. This change is in preparation for the ad hoc polymorphism feature that will require expanding the first argument of a function call just enough to disambiguate amoing all functions with the same name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty clean.
Not many negative tests.
doc/tutorial/tutorial.md
Outdated
extern function log(module: module_t, level: log_level_t, msg: string): () | ||
``` | ||
|
||
The compiler automatically infers these annotations for non-extern functions, so |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems inconsistent, before you said that non-extern functions cannot have side-effects. So what annotations are inferred?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
non-extern functions can invoke extern functions with side effects. I will clarify.
| otherwise = | ||
(rname $ name f) <> "_" <> targ0 <> "_" <> pp (length $ funcArgs f) | ||
where | ||
arg0 = funcArgs f !! 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why can't this go out of bounds?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
arg0
is only evaluated if this is a polymorphic function. Polymorphic functions must take at least one argument (enforced in Validate.hs)
-- recursive functions, as reasoning about them seems tricky otherwise. | ||
and exprArgs && (maybe False (exprIsInjective d (CtxFunc f) (S.fromList $ map (\a -> ArgVar f $ name a) funcArgs)) $ funcDef) | ||
where f@Function{..} = getFunc d exprFunc | ||
exprIsInjective' _ EApply{..} = and exprArgs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what happened to recursive functions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops. I actually implemented support for them but forgot to use it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, we're good, that check just moved above.
| otherwise = t | ||
where t = funcType $ getFunc d f ts | ||
|
||
exprNodeType' _ _ e@EApply{} = error $ "exprNodeType' called with unresolved functio name: " ++ show e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo in error message
-- Attributes do not affect the semantics of the program and can therefore | ||
-- be validated last. | ||
progValidateAttributes d'' | ||
return d'' | ||
|
||
{- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are you still doing this someplace else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, recursive functions are now allowed
//var podsByNode = Aggregate((appliedToGroup), group2map((nodeName, podsOnNode))). | ||
|
||
AppliedToGroupSpan(appliedToGroup, nodeName) :- | ||
AppliedToGroupPod(appliedToGroup, _, nodeName). | ||
// var span = Aggregate((appliedToGroup), group2set(nodeName)). | ||
// var span = Aggregate((appliedToGroup), group`/2set(nodeName)). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo in comment?
This commit implements RFC vmware#670. We already have support for argument polymorphism, e.g.: ``` function vec_length(v: Vec<'X>): usize ``` Here we introduce ad hoc polymorphism, that additionally allows defining functions with the same name for different types, e.g.,: ``` function size(v: Set<'X>): usize {...} function size(v: Map<'X>): usize {...} ``` The compier uses the number of arguments and the type of the _first_ argument to disambiguate the callee: ``` // (1) function size(v: Vec<'X>): usize {...} // OK: different number of arguments function size(v: Vec<'X>, foo: 'Y): usize {...} // OK: different type of the first argument. function size(v: Set<'X>): usize {...} // ERROR: conflicts with (1) function size(v: Vec<bool>): usize {...} ``` This improves DDlog code in a coupe of ways: - We no longer need to prefix function names with type or module names - Once we support object-oriented function call notation, it will be nicer to write `x.length()` vs `x.vec_length()` - We will be able to get rid of the horrible hack with string conversions functions where the function name is computed by addint `2string` to the type name, and just name all of them `to_string()`. Implementation details: - Within a module, we always report conflicts regardless of whether conflicting functions are invoked anywhere in the program. When conflicting declarations appear in different modules, we postpone the check until the user makes a function call that cannot be unambiguously resolved to one of the candidates, i.e., we won't complain even if the user imports both modules unqualified unless they try calling the function. - Rust does not allow functions with the same name unless they appear inside different modules or impl blocks. Both options don't work well for us. Instead the compiler mangles names of polymorphic functions by adding the type of the first argument and the number of arguments to the name. This is invisible to the user, except in extern functions, which must be named-mangled manually. This is ugly and fragile; hence the preferred solution is to make sure that extern function names are unique, and define aliases to them in DDlog. E.g, in std.dl we keep old function names like string_substr and add new functions, e.g., substr(string,...) that invoke them. This commit also adds support for recursive functions.
Implements #674
Details - in commit messages.
Also adds support for recursive functions along the way.
Not in this PR:
I will work on these next.