Return the constr (ConstrBuilder), which is functional, since doing this assumes the original argument is immutable (meaning that we implicitly modify a local clone)
Pass the constr as a mutable reference (&mut ConstrBuilder) and modify this directly.
It is sloppy to use both.
Noticed this when writing the constraint_cases function in #382
Proposed change
It would be detrimental to performance I think to create a copy each time.
So I would propose that we modify the Constrained type to only include the environment.
The environment should be immutable as we don't want any chance of contamination.
In the case of the ConstrBuilder, it internally maintains several distinct sets of constraints to prevent contaminiation.
tl;dr:
Change Constrained from:
pub type Constrained<T = (ConstrBuilder, Environment)> = Result<T, Vec<TypeErr>>;
to:
pub type Constrained<T = Environment> = Result<T, Vec<TypeErr>>;
Summary of current issue
It's a bit odd that we in the check stage both:
ConstrBuilder
), which is functional, since doing this assumes the original argument is immutable (meaning that we implicitly modify a local clone)&mut ConstrBuilder
) and modify this directly.It is sloppy to use both. Noticed this when writing the
constraint_cases
function in #382Proposed change
It would be detrimental to performance I think to create a copy each time. So I would propose that we modify the
Constrained
type to only include the environment. The environment should be immutable as we don't want any chance of contamination. In the case of theConstrBuilder
, it internally maintains several distinct sets of constraints to prevent contaminiation.tl;dr:
Constrained
from:pub type Constrained<T = (ConstrBuilder, Environment)> = Result<T, Vec<TypeErr>>;
to:pub type Constrained<T = Environment> = Result<T, Vec<TypeErr>>;