language-servers/roc
1
0
Fork 0
mirror of https://github.com/roc-lang/roc.git synced 2025-08-03 19:58:18 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

Merge pull request #3736 from rtfeldman/i3687

Browse source 
Creation of a record whose type has an optional value is an error
This commit is contained in:
Folkert de Vries 2022-08-11 15:51:41 +02:00 • committed by GitHub
commit 0798f787c5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
18 changed files with 189 additions and 27 deletions
Download patch file Download diff file Expand all files Collapse all files

4
crates/ast/src/lang/core/types.rs
View file

@ -419,6 +419,10 @@ pub fn to_type2<'a>(
let field_id = env.pool.add(field.into_inner());
RecordField::Optional(field_id)
}
RecordField::RigidOptional(_) => {
let field_id = env.pool.add(field.into_inner());
RecordField::RigidOptional(field_id)
}
RecordField::Demanded(_) => {
let field_id = env.pool.add(field.into_inner());
RecordField::Demanded(field_id)

9
crates/ast/src/solve_type.rs
View file

@ -843,6 +843,15 @@ fn type_to_variable<'a>(
cached,
mempool.get(*type_id),
)),
RigidOptional(type_id) => RigidOptional(type_to_variable(
arena,
mempool,
subs,
rank,
pools,
cached,
mempool.get(*type_id),
)),
Demanded(type_id) => Demanded(type_to_variable(
arena,
mempool,

2
crates/compiler/can/src/annotation.rs
View file

@ -1220,7 +1220,7 @@ fn can_assigned_fields<'a>(
);
let label = Lowercase::from(field_name.value);
field_types.insert(label.clone(), Optional(field_type));
field_types.insert(label.clone(), RigidOptional(field_type));
break 'inner label;
}

4
crates/compiler/mono/src/layout.rs
View file

@ -2260,7 +2260,7 @@ fn layout_from_flat_type<'a>(
RecordField::Required(field_var) | RecordField::Demanded(field_var) => {
sortables.push((label, Layout::from_var(env, field_var)?));
}
RecordField::Optional(_) => {
RecordField::Optional(_) | RecordField::RigidOptional(_) => {
// drop optional fields
}
}
@ -2361,7 +2361,7 @@ fn sort_record_fields_help<'a>(
let layout = Layout::from_var(env, v)?;
sorted_fields.push((label, v, Ok(layout)));
}
RecordField::Optional(v) => {
RecordField::Optional(v) | RecordField::RigidOptional(v) => {
let layout = Layout::from_var(env, v)?;
sorted_fields.push((label, v, Err(layout)));
}

2
crates/compiler/mono/src/layout_soa.rs
View file

@ -804,7 +804,7 @@ impl Layout {
let it = slice.indices().zip(fields.iter_all());
for (target_index, (_, field_index, var_index)) in it {
match subs.record_fields[field_index.index as usize] {
RecordField::Optional(_) => {
RecordField::Optional(_) | RecordField::RigidOptional(_) => {
// do nothing
}
RecordField::Required(_) | RecordField::Demanded(_) => {

29
crates/compiler/solve/src/solve.rs
View file

@ -32,7 +32,7 @@ use roc_types::subs::{
use roc_types::types::Type::{self, *};
use roc_types::types::{
gather_fields_unsorted_iter, AliasCommon, AliasKind, Category, OptAbleType, OptAbleVar, Reason,
TypeExtension, Uls,
RecordField, TypeExtension, Uls,
};
use roc_unify::unify::{
unify, unify_introduced_ability_specialization, Env as UEnv, Mode, Obligated,
@ -2240,6 +2240,7 @@ fn type_to_variable<'a>(
Optional(t) => Optional(helper!(t)),
Required(t) => Required(helper!(t)),
Demanded(t) => Demanded(helper!(t)),
RigidOptional(t) => RigidOptional(helper!(t)),
}
};
@ -3486,11 +3487,35 @@ fn deep_copy_var_help(
let new_variables =
copy_sequence!(fields.len(), fields.iter_variables());
// When copying a let-generalized record to a specialized region, rigid
// optionals just become optionals.
let field_types = subs.get_subs_slice(fields.record_fields());
let has_rigid_optional_field = field_types
.iter()
.any(|f| matches!(f, RecordField::RigidOptional(..)));
let new_field_types_start = if has_rigid_optional_field {
let field_types = field_types.to_vec();
let slice = SubsSlice::extend_new(
&mut subs.record_fields,
field_types.into_iter().map(|f| match f {
RecordField::RigidOptional(()) => RecordField::Optional(()),
RecordField::Demanded(_)
| RecordField::Required(_)
| RecordField::Optional(_) => f,
}),
);
slice.start
} else {
fields.field_types_start
};
RecordFields {
length: fields.length,
field_names_start: fields.field_names_start,
variables_start: new_variables.start,
field_types_start: fields.field_types_start,
field_types_start: new_field_types_start,
}
};

3
crates/compiler/solve/tests/solve_expr.rs
View file

@ -4053,8 +4053,7 @@ mod solve_expr {
{ x, y }
"#
),
// TODO: when structural types unify with alias, they should take the alias name
"{ x : I64, y ? [False, True] }* -> { x : I64, y : Bool }",
"{ x : I64, y ? Bool }* -> { x : I64, y : Bool }",
);
}

1
crates/compiler/types/src/pretty_print.rs
View file

@ -1104,6 +1104,7 @@ fn write_flat_type<'a>(
Optional(_) => buf.push_str(" ? "),
Required(_) => buf.push_str(" : "),
Demanded(_) => buf.push_str(" : "),
RigidOptional(_) => buf.push_str(" ? "),
};
write_content(

8
crates/compiler/types/src/subs.rs
View file

@ -921,9 +921,10 @@ fn subs_fmt_flat_type(this: &FlatType, subs: &Subs, f: &mut fmt::Formatter) -> f
let (it, new_ext) = fields.sorted_iterator_and_ext(subs, *ext);
for (name, content) in it {
let separator = match content {
RecordField::Optional(_) => '?',
RecordField::Required(_) => ':',
RecordField::Demanded(_) => ':',
RecordField::Optional(_) => "?",
RecordField::RigidOptional(_) => "r?",
RecordField::Required(_) => ":",
RecordField::Demanded(_) => ":",
};
write!(
f,
@ -3789,6 +3790,7 @@ fn flat_type_to_err_type(
Optional(_) => Optional(error_type),
Required(_) => Required(error_type),
Demanded(_) => Demanded(error_type),
RigidOptional(_) => RigidOptional(error_type),
};
err_fields.insert(label, err_record_field);

28
crates/compiler/types/src/types.rs
View file

@ -29,13 +29,16 @@ const GREEK_LETTERS: &[char] = &[
/// Cannot unify with an Optional field, but can unify with a Required field
/// - Required: introduced by record literals and type annotations.
/// Can unify with Optional and Demanded
/// - Optional: introduced by pattern matches and annotations.
/// - Optional: introduced by pattern matches, e.g. { x ? "" } ->
/// Can unify with Required, but not with Demanded
/// - RigidOptional: introduced by annotations, e.g. { x ? Str}
/// Can only unify with Optional, to prevent a required field being typed as Optional
#[derive(PartialEq, Eq, Clone, Hash)]
pub enum RecordField<T> {
Optional(T),
Required(T),
Demanded(T),
Required(T),
Optional(T),
RigidOptional(T),
}
impl<T: Copy> Copy for RecordField<T> {}
@ -48,6 +51,7 @@ impl<T: fmt::Debug> fmt::Debug for RecordField<T> {
Optional(typ) => write!(f, "Optional({:?})", typ),
Required(typ) => write!(f, "Required({:?})", typ),
Demanded(typ) => write!(f, "Demanded({:?})", typ),
RigidOptional(typ) => write!(f, "RigidOptional({:?})", typ),
}
}
}
@ -60,6 +64,7 @@ impl<T> RecordField<T> {
Optional(t) => t,
Required(t) => t,
Demanded(t) => t,
RigidOptional(t) => t,
}
}
@ -70,6 +75,7 @@ impl<T> RecordField<T> {
Optional(t) => t,
Required(t) => t,
Demanded(t) => t,
RigidOptional(t) => t,
}
}
@ -80,6 +86,7 @@ impl<T> RecordField<T> {
Optional(t) => t,
Required(t) => t,
Demanded(t) => t,
RigidOptional(t) => t,
}
}
@ -92,6 +99,7 @@ impl<T> RecordField<T> {
Optional(t) => Optional(f(t)),
Required(t) => Required(f(t)),
Demanded(t) => Demanded(f(t)),
RigidOptional(t) => RigidOptional(f(t)),
}
}
}
@ -104,6 +112,7 @@ impl RecordField<Type> {
Optional(typ) => typ.substitute(substitutions),
Required(typ) => typ.substitute(substitutions),
Demanded(typ) => typ.substitute(substitutions),
RigidOptional(typ) => typ.substitute(substitutions),
}
}
@ -119,6 +128,7 @@ impl RecordField<Type> {
Optional(typ) => typ.substitute_alias(rep_symbol, rep_args, actual),
Required(typ) => typ.substitute_alias(rep_symbol, rep_args, actual),
Demanded(typ) => typ.substitute_alias(rep_symbol, rep_args, actual),
RigidOptional(typ) => typ.substitute_alias(rep_symbol, rep_args, actual),
}
}
@ -137,6 +147,7 @@ impl RecordField<Type> {
Optional(typ) => typ.instantiate_aliases(region, aliases, var_store, introduced),
Required(typ) => typ.instantiate_aliases(region, aliases, var_store, introduced),
Demanded(typ) => typ.instantiate_aliases(region, aliases, var_store, introduced),
RigidOptional(typ) => typ.instantiate_aliases(region, aliases, var_store, introduced),
}
}
@ -147,6 +158,7 @@ impl RecordField<Type> {
Optional(typ) => typ.contains_symbol(rep_symbol),
Required(typ) => typ.contains_symbol(rep_symbol),
Demanded(typ) => typ.contains_symbol(rep_symbol),
RigidOptional(typ) => typ.contains_symbol(rep_symbol),
}
}
pub fn contains_variable(&self, rep_variable: Variable) -> bool {
@ -156,6 +168,7 @@ impl RecordField<Type> {
Optional(typ) => typ.contains_variable(rep_variable),
Required(typ) => typ.contains_variable(rep_variable),
Demanded(typ) => typ.contains_variable(rep_variable),
RigidOptional(typ) => typ.contains_variable(rep_variable),
}
}
}
@ -564,6 +577,9 @@ impl fmt::Debug for Type {
RecordField::Optional(_) => write!(f, "{:?} ? {:?}", label, field_type)?,
RecordField::Required(_) => write!(f, "{:?} : {:?}", label, field_type)?,
RecordField::Demanded(_) => write!(f, "{:?} : {:?}", label, field_type)?,
RecordField::RigidOptional(_) => {
write!(f, "{:?} ? {:?}", label, field_type)?
}
}
if any_written_yet {
@ -1595,6 +1611,7 @@ fn variables_help(tipe: &Type, accum: &mut ImSet<Variable>) {
Optional(x) => variables_help(x, accum),
Required(x) => variables_help(x, accum),
Demanded(x) => variables_help(x, accum),
RigidOptional(x) => variables_help(x, accum),
};
}
@ -1736,6 +1753,7 @@ fn variables_help_detailed(tipe: &Type, accum: &mut VariableDetail) {
Optional(x) => variables_help_detailed(x, accum),
Required(x) => variables_help_detailed(x, accum),
Demanded(x) => variables_help_detailed(x, accum),
RigidOptional(x) => variables_help_detailed(x, accum),
};
}
@ -2312,7 +2330,7 @@ fn write_error_type_help(
buf.push_str(label.as_str());
let content = match field {
Optional(content) => {
Optional(content) | RigidOptional(content) => {
buf.push_str(" ? ");
content
}
@ -2466,7 +2484,7 @@ fn write_debug_error_type_help(error_type: ErrorType, buf: &mut String, parens:
buf.push_str(label.as_str());
let content = match field {
Optional(content) => {
Optional(content) | RigidOptional(content) => {
buf.push_str(" ? ");
content
}

11
crates/compiler/unify/src/unify.rs
View file

@ -1772,8 +1772,10 @@ fn unify_shared_fields<M: MetaCollector>(
// Unification of optional fields
//
// Demanded does not unify with Optional
// RigidOptional does not unify with Required or Demanded
// Unifying Required with Demanded => Demanded
// Unifying Optional with Required => Required
// Unifying Optional with RigidOptional => RigidOptional
// Unifying X with X => X
let actual = match (actual, expected) {
(Demanded(_), Optional(_)) | (Optional(_), Demanded(_)) => {
@ -1787,6 +1789,15 @@ fn unify_shared_fields<M: MetaCollector>(
(Required(val), Optional(_)) => Required(val),
(Optional(val), Required(_)) => Required(val),
(Optional(val), Optional(_)) => Optional(val),
(RigidOptional(val), Optional(_)) | (Optional(_), RigidOptional(val)) => {
RigidOptional(val)
}
(RigidOptional(_), Demanded(_) | Required(_))
| (Demanded(_) | Required(_), RigidOptional(_)) => {
// this is an error, but we continue to give better error messages
continue;
}
(RigidOptional(val), RigidOptional(_)) => RigidOptional(val),
};
matching_fields.push((name, actual));

2
crates/glue/src/types.rs
View file

@ -698,7 +698,7 @@ fn add_type_help<'a>(
RecordField::Required(field_var) | RecordField::Demanded(field_var) => {
Some((label.to_string(), field_var))
}
RecordField::Optional(_) => {
RecordField::Optional(_) | RecordField::RigidOptional(_) => {
// drop optional fields
None
}

17
crates/reporting/src/error/type.rs
View file

@ -2272,6 +2272,9 @@ fn to_doc_help<'b>(
Parens::Unnecessary,
v,
)),
RecordField::RigidOptional(v) => RecordField::RigidOptional(
to_doc_help(ctx, alloc, Parens::Unnecessary, v),
),
RecordField::Required(v) => RecordField::Required(to_doc_help(
ctx,
alloc,
@ -2738,6 +2741,7 @@ fn diff_record<'b>(
alloc.string(field.as_str().to_string()),
match t1 {
RecordField::Optional(_) => RecordField::Optional(diff.left),
RecordField::RigidOptional(_) => RecordField::RigidOptional(diff.left),
RecordField::Required(_) => RecordField::Required(diff.left),
RecordField::Demanded(_) => RecordField::Demanded(diff.left),
},
@ -2747,6 +2751,7 @@ fn diff_record<'b>(
alloc.string(field.as_str().to_string()),
match t2 {
RecordField::Optional(_) => RecordField::Optional(diff.right),
RecordField::RigidOptional(_) => RecordField::RigidOptional(diff.right),
RecordField::Required(_) => RecordField::Required(diff.right),
RecordField::Demanded(_) => RecordField::Demanded(diff.right),
},
@ -2754,7 +2759,15 @@ fn diff_record<'b>(
status: {
match (&t1, &t2) {
(RecordField::Demanded(_), RecordField::Optional(_))
| (RecordField::Optional(_), RecordField::Demanded(_)) => match diff.status {
| (RecordField::Optional(_), RecordField::Demanded(_))
| (
RecordField::Demanded(_) | RecordField::Required(_),
RecordField::RigidOptional(_),
)
| (
RecordField::RigidOptional(_),
RecordField::Demanded(_) | RecordField::Required(_),
) => match diff.status {
Status::Similar => {
Status::Different(vec![Problem::OptionalRequiredMismatch(
field.clone(),
@ -3190,7 +3203,7 @@ mod report_text {
RecordField::Required(field) => {
field_name.append(alloc.text(" : ")).append(field)
}
RecordField::Optional(field) => {
RecordField::Optional(field) | RecordField::RigidOptional(field) => {
field_name.append(alloc.text(" ? ")).append(field)
}
}

80
crates/reporting/tests/test_reporting.rs
View file

@ -6602,6 +6602,25 @@ All branches in an `if` must have the same type!
{ inputs ? List Str }
Tip: To extract the `.inputs` field it must be non-optional, but the
type says this field is optional. Learn more about optional fields at
TODO.
── TYPE MISMATCH ───────────────────────────────────────── /code/proj/Main.roc ─
This 1st argument to `job` has an unexpected type:
9│ job { inputs: ["build", "test"] }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The argument is a record of type:
{ inputs : List Str }
But `job` needs its 1st argument to be:
{ inputs ? List Str }
Tip: To extract the `.inputs` field it must be non-optional, but the
type says this field is optional. Learn more about optional fields at
TODO.
@ -10356,4 +10375,65 @@ All branches in an `if` must have the same type!
The branches must be cases of the `when` condition's type!
"###
);
test_report!(
create_value_with_optional_record_field_type,
indoc!(
r#"
f : {a: Str, b ? Str}
f = {a: "b", b: ""}
f
"#
),
@r###"
── TYPE MISMATCH ───────────────────────────────────────── /code/proj/Main.roc ─
Something is off with the body of the `f` definition:
4│ f : {a: Str, b ? Str}
5│ f = {a: "b", b: ""}
^^^^^^^^^^^^^^^
The body is a record of type:
{ a : Str, b : Str }
But the type annotation on `f` says it should be:
{ a : Str, b ? Str }
Tip: To extract the `.b` field it must be non-optional, but the type
says this field is optional. Learn more about optional fields at TODO.
"###
);
test_report!(
create_value_with_conditionally_optional_record_field_type,
indoc!(
r#"
f : {a: Str, b ? Str}
f = if True then {a: ""} else {a: "b", b: ""}
f
"#
),
@r###"
── TYPE MISMATCH ───────────────────────────────────────── /code/proj/Main.roc ─
Something is off with the `then` branch of this `if` expression:
4│ f : {a: Str, b ? Str}
5│ f = if True then {a: ""} else {a: "b", b: ""}
^^^^^^^
The 1st branch is a record of type:
{ a : Str }
But the type annotation on `f` says it should be:
{ a : Str, b ? Str }
Tip: Looks like the b field is missing.
"###
);
}

4
examples/breakout/platform/Cargo.lock generated
View file

@ -1053,7 +1053,7 @@ checksum = "dfa686283ad6dd069f105e5ab091b04c62850d3e4cf5d67debad1933f55023df"
[[package]]
name = "host"
version = "0.1.0"
version = "0.0.1"
dependencies = [
"arrayvec",
"bytemuck",
@ -2089,7 +2089,7 @@ checksum = "f1382d1f0a252c4bf97dc20d979a2fdd05b024acd7c2ed0f7595d7817666a157"
[[package]]
name = "roc_std"
version = "0.1.0"
version = "0.0.1"
dependencies = [
"arrayvec",
"static_assertions",

4
examples/false-interpreter/platform/Cargo.lock generated
View file

@ -10,7 +10,7 @@ checksum = "8da52d66c7071e2e3fa2a1e5c6d088fec47b593032b254f5e980de8ea54454d6"
[[package]]
name = "host"
version = "0.1.0"
version = "0.0.1"
dependencies = [
"libc",
"roc_std",
@ -24,7 +24,7 @@ checksum = "a1fa8cddc8fbbee11227ef194b5317ed014b8acbf15139bd716a18ad3fe99ec5"
[[package]]
name = "roc_std"
version = "0.1.0"
version = "0.0.1"
dependencies = [
"arrayvec",
"static_assertions",

4
examples/gui/platform/Cargo.lock generated
View file

@ -1053,7 +1053,7 @@ checksum = "dfa686283ad6dd069f105e5ab091b04c62850d3e4cf5d67debad1933f55023df"
[[package]]
name = "host"
version = "0.1.0"
version = "0.0.1"
dependencies = [
"arrayvec",
"bytemuck",
@ -2089,7 +2089,7 @@ checksum = "f1382d1f0a252c4bf97dc20d979a2fdd05b024acd7c2ed0f7595d7817666a157"
[[package]]
name = "roc_std"
version = "0.1.0"
version = "0.0.1"
dependencies = [
"arrayvec",
"static_assertions",

4
examples/platform-switching/rust-platform/Cargo.lock generated
View file

@ -10,7 +10,7 @@ checksum = "8da52d66c7071e2e3fa2a1e5c6d088fec47b593032b254f5e980de8ea54454d6"
[[package]]
name = "host"
version = "0.1.0"
version = "0.0.1"
dependencies = [
"libc",
"roc_std",
@ -24,7 +24,7 @@ checksum = "56d855069fafbb9b344c0f962150cd2c1187975cb1c22c1522c240d8c4986714"
[[package]]
name = "roc_std"
version = "0.1.0"
version = "0.0.1"
dependencies = [
"arrayvec",
"static_assertions",