Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 24 additions & 20 deletions manual/src/features/augmented-assignment.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,49 +39,53 @@ source order. The augmented assignment expression itself evaluates to `()`.

## Type checking

An augmented assignment with an in-place operator such as `++=` never changes
the type of its target. If the right-hand side would force a type change the
program is rejected:
`a op= b` is checked like `a = a op b`: the operator has to take both operands,
and what it returns has to fit `a`. An in-place operator such as `++=` is used
where it does the same thing, so it never changes the type of its target. A
right-hand side whose element type differs is rejected:

```ndc,fails
let values = [1];
values ++= ["two"]; // error: mismatched types: found List<String> but expected List<Int>
values ++= ["two"]; // error: No function called '++' found that matches the arguments 'List<Int>, List<String>'
```

The right-hand side has to *provably* fit. A wider element type is not enough,
because the operator copies the values across without checking them:
A wider element type is not enough either, as `++` keeps one element type:

```ndc,fails
let values: List<Int> = [1];
let rhs: List<Number> = [0.5n];
values ++= rhs; // error: mismatched types: found List<Number> but expected List<Int>
values ++= rhs; // error: No function called '++' found that matches the arguments 'List<Int>, List<Number>'
```

The same goes for an operand whose type is unknown. `Any` says nothing about
what the value holds, and no later step re-checks it, so it is rejected too:
An operand of type `Any` is checked as the program runs, as in `a = a op b`.
What the operator makes is checked where it's written back, so a value that
doesn't fit fails there and leaves the target as it was. As nothing says the
operand fits before it's checked, the operator makes a new value rather than
changing the target in place:

```ndc,fails
```ndc
fn opaque(x: Any) => x;
let values = [1];
values ++= opaque([2, 3]); // error: mismatched types: found Any but expected List<Int>
let same = values;
values ++= opaque([2, 3]);
print(values, same); // [1,2,3] [1]
```

[Cast](casts.md) the operand to say what it holds. The cast checks the value, so
the assignment gets its guarantee from the cast site:

```ndc
```ndc,fails
fn opaque(x: Any) => x;
let values = [1];
values ++= opaque([2, 3]) as List<Int>;
print(values); // [1,2,3]
values ++= opaque([0.5]); // error: expected List<Int> but found List<Any>
```

A cast that does not hold fails there rather than corrupting the target:
[Cast](casts.md) the operand to say what it holds, which checks it at the cast
and lets `++=` change the target in place:

```ndc,fails
```ndc
fn opaque(x: Any) => x;
let values = [1];
values ++= opaque([0.5]) as List<Int>; // error: cannot cast List<Float> to List<Int>
let same = values;
values ++= opaque([2, 3]) as List<Int>;
print(values, same); // [1,2,3] [1,2,3]
```

Annotate the target with `Any` to opt into heterogeneous contents:
Expand Down
8 changes: 4 additions & 4 deletions manual/src/features/casts.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,10 @@ let values: List<Any> = [1, 2, 3];
print((values as List<Int>).sum()); // 6
```

A specialized `op=` mutates its target in place and keeps its type, so it only
accepts a right operand that provably fits. A value of unknown type does not,
and a cast gets the elements checked instead of letting them slip into the
target unverified:
A specialized `op=` such as `++=` changes its target in place, so it's only
used with a right operand that provably fits. With a value of unknown type,
`a op= b` makes a new value like `a = a op b` does; a cast checks the elements
first, so the target can be changed in place:

```ndc
fn opaque(x) => x;
Expand Down
95 changes: 26 additions & 69 deletions ndc_analyser/src/analyser/assignments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,25 +23,26 @@ impl Analyser {
) -> StaticType {
let left_type = self.analyse_assignment_target(l_value, span);
let right_type = self.analyse_expr(r_value);
// `a op= b` is `a = op(a, b)`. Where the operands aren't fully known
// yet, the operator waits for them like any call, and the in-place
// overload is picked along with it.
// `a op= b` is `a = op(a, b)`: the operator is a call like any other,
// and the in-place overload is picked along with it where it does the
// same (see `record_pick`).
let (left_type, right_type) = (
self.vars.resolve(&left_type),
self.vars.resolve(&right_type),
);
if (left_type.has_vars() || right_type.has_vars())
&& !self
.overloads()
.reachable_symbols(&self.resolver.augmented_candidates(id).operator)
.is_empty()
if !self
.overloads()
.reachable_symbols(&self.resolver.augmented_candidates(id).operator)
.is_empty()
{
let result = self.constrain_call(
CallSite::OpAssignment(id),
vec![left_type.clone(), right_type],
// What the operator returns is checked where it's written
// back, when the program picks the operator as it runs.
CallNodes {
arguments: vec![None, Some(r_value.id)],
result: None,
result: Some(l_value.id),
Comment on lines 43 to +45

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Check an Any-typed target before calling a resolved operator

When the target has type Any and only one ordinary operator overload is viable, resolve_call_now promotes the dynamic pick to a resolved call, but expected_of_any cannot emit a check for the first argument because its node is None. For example, fn choose(a: Int, b: Int) => a; let x: Any = "wrong"; x choose= 1; can pass a String directly into the Int parameter, whereas x = choose(x, 1) checks and rejects it. Previously, fully known operand types used dynamic dispatch here, which checked the actual left operand before selecting the overload. Preserve that check when routing augmented assignments through ordinary call constraints; checking the result at writeback does not validate the argument before the function executes.

Useful? React with 👍 / 👎.

},
CallKind::Operator,
operation,
Expand All @@ -50,80 +51,36 @@ impl Analyser {
self.validate_assignment_write(l_value, &left_type, &result, span);
return StaticType::unit();
}
let written = self.op_assignment_now(id, &left_type, &right_type, operation, span);
let written = self.in_place_only(id, &left_type, &right_type, operation, span);
self.validate_assignment_write(l_value, &left_type, &written, span);
StaticType::unit()
}

/// Picks what `a op= b` with operands of these types does and returns
/// the type of the value written back to `a`. An in-place overload
/// such as `++=` is used where it keeps `a`'s type, and otherwise `op`
/// makes a new value.
pub(super) fn op_assignment_now(
/// Picks the in-place overload `a op= b` calls when there's no operator
/// `op` to fall back on, and returns the type of the value written back
/// to `a`, which an in-place overload doesn't change.
fn in_place_only(
&mut self,
id: NodeId,
left_type: &StaticType,
right_type: &StaticType,
operation: &str,
span: Span,
) -> StaticType {
let arg_types = vec![left_type.clone(), right_type.clone()];

let ResolvedCall {
binding: assign_binding,
return_type: assign_return,
} = self.overloads().resolve_call(
let arguments = [left_type.clone(), right_type.clone()];
let ResolvedCall { binding, .. } = self.overloads().resolve_call(
&self.resolver.augmented_candidates(id).in_place,
&arg_types,
CallKind::Operator,
);
let ResolvedCall {
binding: op_binding,
return_type: op_return,
} = self.overloads().resolve_call(
&self.resolver.augmented_candidates(id).operator,
&arg_types,
&arguments,
CallKind::Operator,
);
let has_op_binding = !matches!(op_binding, Binding::None);
let assign_is_eligible = Self::augmented_rhs_is_compatible(left_type, right_type);

let writeback_type = match assign_binding {
Binding::Resolved(candidate) if assign_is_eligible => {
self.record_augmented_pick(id, Binding::Resolved(candidate));
None
}
Binding::Dynamic(assign_candidates) if assign_is_eligible => {
let binding = Self::in_place_then_operators(assign_candidates, op_binding.clone());
let result_type = if has_op_binding {
assign_return.join(&op_return)
} else {
assign_return
};
self.record_augmented_pick(id, binding);
Some(result_type)
}
Binding::Resolved(_) | Binding::Dynamic(_) => {
// A specialized mutation exists, but using it would
// change the concrete left type. Reject it here rather
// than falling through to an ordinary operation whose
// erased return type could widen the same target.
self.emit(AnalysisError::mismatched_types(right_type, left_type, span));
Some(StaticType::Any)
}
Binding::None if has_op_binding => {
self.record_augmented_pick(id, op_binding);
Some(op_return)
}
Binding::None => {
self.emit(AnalysisError::function_not_found(
operation, &arg_types, span,
));
Some(StaticType::Any)
}
};

writeback_type.unwrap_or_else(|| left_type.clone())
if matches!(binding, Binding::None) {
self.emit(AnalysisError::function_not_found(
operation, &arguments, span,
));
return StaticType::Any;
}
self.record_augmented_pick(id, binding);
left_type.clone()
}

/// What `a op= b` does when the program picks as it runs: the in-place
Expand Down
86 changes: 38 additions & 48 deletions ndc_analyser/src/analyser/solver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,8 +418,13 @@ impl Analyser {
.map(|argument| self.vars.resolve(argument))
.collect::<Vec<_>>();
// An argument of type `Any` is checked where it's passed, so the
// program can call `picked` directly.
let binding = Binding::Resolved(Candidate::Scalar(picked));
// program can call `picked` directly, unless one has nowhere to be
// checked.
let binding = if Self::checks_every_any(&arguments, &call.nodes.arguments) {
Binding::Resolved(Candidate::Scalar(picked))
} else {
Binding::Dynamic(vec![Candidate::Scalar(picked)])
};
if cfg!(debug_assertions) {
self.early_picks.push(EarlyPick::Call {
candidates: call.candidates.clone(),
Expand Down Expand Up @@ -1271,9 +1276,6 @@ impl Analyser {
}

if arguments.iter().all(|argument| !argument.has_vars()) {
if let (CallSite::OpAssignment(id), [left, right]) = (call.site, &arguments[..]) {
return Some(self.op_assignment_now(id, left, right, &call.name, call.span));
}
return Some(self.resolve_call_now(call, &arguments));
}

Expand Down Expand Up @@ -1492,51 +1494,23 @@ impl Analyser {
merged
}

/// [`Overloads::resolve_call`], with the overloads it leaves the program
/// to pick from as it runs cut to the ones that take the arguments with
/// each type parameter one type. Picking looks at each argument on its
/// own, so `[]=(List<T>, Int, T)` looks like it takes a String for a
/// `List<Int>` when the index is `Any`, but it can't.
/// Whether every argument of type `Any` has a node in `nodes` to be checked
/// at where it's passed.
fn checks_every_any(arguments: &[StaticType], nodes: &[Option<NodeId>]) -> bool {
arguments
.iter()
.zip(nodes)
.all(|(argument, node)| node.is_some() || *argument != StaticType::Any)
}

/// [`Overloads::resolve_call`] over the types as they're known now.
pub(super) fn resolve_overloads(
&mut self,
candidates: &Candidates,
arguments: &[StaticType],
kind: CallKind,
) -> ResolvedCall {
let mut resolved = self.overloads().resolve_call(candidates, arguments, kind);
// An operator may apply an overload to each element of a tuple
// instead, which takes what the elements are.
if kind == CallKind::Operator {
return resolved;
}
if let Binding::Dynamic(picked) = &resolved.binding {
let picked = picked.clone();
// What a function takes and returns isn't checked as the program
// runs, so it's left out: a lambda that returns `Any` may still be
// what's expected.
let snapshot = self.vars.snapshot();
let arguments = arguments
.iter()
.map(|argument| match self.vars.resolve(argument) {
StaticType::Function { .. } => self.vars.fresh(StaticType::Any),
argument => argument,
})
.collect::<Vec<_>>();
let kept = picked
.into_iter()
.filter(|candidate| match candidate {
Candidate::Scalar(symbol) => self.could_accept(*symbol, &arguments),
Candidate::Vec(_) => true,
})
.collect::<Vec<_>>();
self.vars.rollback(snapshot);
resolved.binding = if kept.is_empty() {
Binding::None
} else {
Binding::Dynamic(kept)
};
}
resolved
self.overloads().resolve_call(candidates, arguments, kind)
}

/// Resolves `call` with the arguments as they are, reporting it when no
Expand All @@ -1548,13 +1522,15 @@ impl Analyser {
} = self.resolve_overloads(&call.candidates, arguments, call.kind);
// A pick left to the program among one function with known
// parameters is a call of it, whose `Any` arguments are checked where
// they're passed. A variable of type `Any` may hold any function, and
// an `Iterable<T>` parameter's kind isn't in its declared type, so the
// program checks those arguments itself.
// they're passed. A variable of type `Any` may hold any function, an
// `Iterable<T>` parameter's kind isn't in its declared type, and an
// `Any` without a node to check it at, as the target of `a op= b`,
// isn't checked where it's passed, so the program checks those itself.
let binding = match binding {
Binding::Dynamic(only)
if let [Candidate::Scalar(only)] = only[..]
&& !self.types.has_shape(only)
&& Self::checks_every_any(arguments, &call.nodes.arguments)
&& let StaticType::Function {
parameters: Some(_),
..
Expand All @@ -1564,6 +1540,20 @@ impl Analyser {
}
binding => binding,
};
// `a op= b` with no operator that takes the operands calls an in-place
// overload if one does, as a set has `-=` but no `-`, which keeps `a`.
if matches!(binding, Binding::None)
&& let CallSite::OpAssignment(id) = call.site
{
let in_place = self.resolver.augmented_candidates(id).in_place.clone();
let in_place = self
.resolve_overloads(&in_place, arguments, call.kind)
.binding;
if !matches!(in_place, Binding::None) {
self.record_augmented_pick(id, in_place);
return arguments[0].clone();
}
}
if matches!(binding, Binding::None) {
self.report_failed_call(call, arguments);
self.record_pick(call.site, binding, arguments);
Expand Down Expand Up @@ -2054,7 +2044,7 @@ impl Analyser {
);
let compatible = Self::augmented_rhs_is_compatible(&arguments[0], &arguments[1]);
let binding = match (in_place.binding, binding) {
(Binding::Resolved(candidate), _) if compatible => Binding::Resolved(candidate),
(Binding::Resolved(candidate), _) => Binding::Resolved(candidate),
// Which one applies is left to the program as it runs,
// which mutates in place where it can.
(Binding::Dynamic(in_place), operators) if compatible => {
Expand Down
Loading
Loading