Support kwargs in method calls #440
Conversation
leina05
force-pushed
the
leina/ship_mixed_args
branch
from
8981e4f to
e00f95d
Compare
| @@ -181,14 +181,6 @@ BuiltinOperator: Operator = { | |||
| }; | |||
|
|
|||
| New: Value = { | |||
| // Keyword argument constructor. | |||
| "new" <literal:Spanned<InstanceLiteralTerm>> => { | |||
There was a problem hiding this comment.
Can we rm the definition of InstanceLiteralTerm on 166?
There was a problem hiding this comment.
Wait... doesn't that mean we can completely get rid of the whole <"Pattern">? (Not for this PR, to verify later!). That would be nice.
| @@ -264,7 +256,7 @@ Exp10<T>: Term = { | |||
| } | |||
|
|
|||
| CallTerm: Value = { | |||
| <SimpleCall>, | |||
There was a problem hiding this comment.
Looks like there might be some additional cleanup of SimpleCall stuff we could do in this file. I'm not sure why Value still accepts a SimpleCall on 491:
pub Value: Value = {
<Number>,
<PolarString>,
<Boolean>,
<Variable>,
<DictionaryTerm>,
<BuiltinOperation>,
<RewrittenOperation>,
<SimpleCall>,
<List<"Term">>,
};
or why we couldn't just collapse the declaration of SimpleCall into Call now that we don't need to refer to SimpleCall separately
There was a problem hiding this comment.
oh wait is the SimpleCall stuff still necessary since we parse Polar predicates (rule heads) the same as method/constructor calls?
There was a problem hiding this comment.
but wait that wouldn't make sense either since (I think) the param: Specializer fields in a Polar predicate require different semantics than method calls, which require all kwargs to be at the end of the arglist.
There was a problem hiding this comment.
We can kill SimpleCall and fold its cases into Call. Thanks for catching this.
There was a problem hiding this comment.
Word of caution: we do support prolog-style person("sam") = person(name) style unification, which is possibly what this syntax is referring to.
There was a problem hiding this comment.
SimpleCall is used for Polar predicates... I don't think it can be folded into Call
There was a problem hiding this comment.
We need it to be separate because of specializers, as Gabe pointed out.
There was a problem hiding this comment.
@plotnick and I talked about this -- we use ParameterList to capture the param: Specializer case in rule heads. We folded SimpleCall into Call and everything seems kosher.
|
@leina05 to update docs as a follow-up |
|
|
||
| - summary of breaking change | ||
| The former accepted positional arguments, and the latter accepted keyword | ||
| arguments. In this release, the two forms have combined their powers, and |
PR checklist: