Skip to content

Commit

Permalink
clean up outdated primitive operator names (#1992)
Browse files Browse the repository at this point in the history
* clean up outdated primitive operator names

PR #1937 recently renamed many primitive operators. It missed some
instances of the previous names, though. In particular:

- error messages
- comments
- an unused function "apply_contract" in core/src/term/mod.rs

While most of these names were only made obsolete in #1937, some of them
have been incorrect for longer, eg "%array_access%" in
core/src/term/pattern/compile.rs and "recordMap" in core/src/term/mod.rs.

I caught as many as I could find. However it's hard to be sure I got all
of them, given that some of the previous names are very general terms
like "value", "fields", "length", and "map".

The full list of renames I identifiend are as follows, formatted as
"<old name> <new name>".

First, the easy cases:

    chng_pol label/flip_polarity
    record_map record/map
    str_trim string/trim
    str_chars string/chars
    str_uppercase string/uppercase
    str_lowercase string/lowercase
    str_length string/length
    str_from to_string
    num_from number/from_string
    enum_from enum/from_string
    str_is_match string/is_match
    str_find string/find
    str_find_all string/find_all
    record_empty_with_tail record/empty_with_tail
    label_push_diag label/push_diag
    enum_unwrap_variant enum/unwrap_variant
    enum_is_variant enum/is_variant
    enum_get_tag enum/get_tag
    apply_contract contract/apply
    array_lazy_app_ctr contract/array_lazy_app
    record_lazy_app_ctr contract/record_lazy_app
    elem_at array/at
    str_split string/split
    str_contains string/contains
    record_insert record/insert
    record_insert_with_opts record/insert_with_opts
    record_remove record/remove
    record_remove_with_opts record/remove_with_opts
    label_with_message label/with_message
    label_with_notes label/with_notes
    label_append_note label/append_note
    str_replace string/replace
    str_replace_regex string/replace_regex
    str_substr string/substr
    record_seal_tail record/seal_tail
    record_unseal_tail record/unseal_tail
    array_slice array/slice

Then the harder cases:

    polarity label/polarity
    go_dom label/go_dom
    go_codom label/go_codom
    go_array label/go_array
    go_dict label/go_dict
    embed enum/embed
    map array/map
    generate array/generate
    length array/length
    fields record/fields
    fields_with_opts record/fields_with_opts
    values record/values
    go_field label/go_field
    has_field record/has_field
    has_field_with_opts record/has_field_with_opts
    field_is_defined record/field_is_defined
    field_is_defined_with_opts record/field_is_defined_with_opts
    lookup_type_variable label/lookup_type_variable
    insert_type_variable label/insert_type_variable

Finally, two cases that I didn't understand and seem to be unused:

    rec_force_op op rec_force
    rec_default_op op rec_default

* address code review comments
  • Loading branch information
jeremyschlatter authored Jul 10, 2024
1 parent 58ef79d commit 090984d
Show file tree
Hide file tree
Showing 7 changed files with 171 additions and 131 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
source: cli/tests/snapshot/main.rs
expression: err
---
error: record_insert: tried to extend a record with the field bar, but it already exists
error: record/insert: tried to extend a record with the field bar, but it already exists


231 changes: 135 additions & 96 deletions core/src/eval/operation.rs

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions core/src/term/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1234,9 +1234,9 @@ pub enum UnaryOp {

/// Typecast an enum to a larger enum type.
///
/// `Embed` is used to upcast enums. For example, if a value `x` has enum type `a | b`, then
/// `embed c x` will have enum type `a | b | c`. It only affects typechecking as at runtime
/// `embed someId` act like the identity.
/// `EnumEmbed` is used to upcast enums. For example, if a value `x` has enum type `a | b`,
/// then `%enum/embed% c x` will have enum type `a | b | c`. It only affects typechecking as at
/// runtime `%enum/embed% someId` acts like the identity function.
EnumEmbed(LocIdent),

/// A specialized primop for match when all patterns are enum tags. In that case, instead of
Expand All @@ -1257,7 +1257,7 @@ pub enum UnaryOp {
///
/// The mapped function must take two arguments, the name of the field as a string, and the
/// content of the field. `RecordMap` then replaces the content of each field by the result of
/// the function: i.e., `recordMap f {a=2;}` evaluates to `{a=(f "a" 2);}`.
/// the function: i.e., `%record/map% f {a=2;}` evaluates to `{a=(f "a" 2);}`.
RecordMap,

/// Inverse the polarity of a label.
Expand Down
32 changes: 16 additions & 16 deletions core/src/term/pattern/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ fn remove_from_rest(rest_field: LocIdent, field: LocIdent, bindings_id: LocIdent
/// More precisely, [with_default_value] generates the following code:
///
/// ```nickel
/// if !(%field_is_defined% "<field>" record_id) then
/// if !(%record/field_is_defined% "<field>" record_id) then
/// if %record/has_field% "<field>" record_id then
/// record_id & { "<field>" = default }
/// else
Expand Down Expand Up @@ -363,14 +363,14 @@ impl CompilePart for RecordPattern {
// - initial accumulator is `%record/insert% "<REST_FIELD>" bindings_id value_id`
// >
//
// # If there is a default value, we must set it before the %field_is_defined% check below,
// # If there is a default value, we must set it before the %record/field_is_defined% check below,
// # because the default acts like if the original matched value always have this field
// # defined
// <if field.default.is_some()>
// let value_id = <with_default_value value_id field default> in
// <end if>
//
// if %field_is_defined% field value_id then
// if %record/field_is_defined% field value_id then
// <if !field_pat.annotation.is_empty()>
// let value_id = value_id & { "<field>" | <field_pat.annotation> } in
// <end if>
Expand Down Expand Up @@ -429,14 +429,14 @@ impl CompilePart for RecordPattern {
// - initial accumulator is `%record/insert% "<REST>" bindings_id value_id`
// >
//
// # If there is a default value, we must set it before the %field_is_defined% check below,
// # If there is a default value, we must set it before the %record/field_is_defined% check below,
// # because the default acts like if the original matched value always have this field
// # defined
// <if field.default.is_some()>
// let value_id = <with_default_value value_id field default> in
// <end if>
//
// if %field_is_defined% field value_id then
// if %record/field_is_defined% field value_id then
// # If the field is present, we apply the potential contracts coming from user-provided
// # annotations before anything else. We just offload the actual work to `&`
// <if !field_pat.annotation.is_empty() >
Expand Down Expand Up @@ -514,7 +514,7 @@ impl CompilePart for RecordPattern {
binding_cont_let
};

// %field_is_defined% field value_id
// %record/field_is_defined% field value_id
let has_field = make::op2(
BinaryOp::RecordFieldIsDefined(RecordOpKind::ConsiderAllFields),
Term::Str(field.label().into()),
Expand Down Expand Up @@ -623,7 +623,7 @@ impl CompilePart for RecordPattern {
impl CompilePart for ArrayPattern {
// Compilation of an array pattern.
//
// let value_len = %array_length% value_id in
// let value_len = %array/length% value_id in
//
// <if self.is_open()>
// if %typeof% value_id == 'Array && value_len >= <self.patterns.len()>
Expand All @@ -641,7 +641,7 @@ impl CompilePart for ArrayPattern {
// if local_bindings_id == null then
// null
// else
// let local_value_id = %array_access% <idx> value_id in
// let local_value_id = %array/at% <idx> value_id in
// <elem_pat.compile_part(local_value_id, local_bindings_id)>
//
// <end fold>
Expand Down Expand Up @@ -673,7 +673,7 @@ impl CompilePart for ArrayPattern {
// if local_bindings_id == null then
// null
// else
// let local_value_id = %array_access% <idx> value_id in
// let local_value_id = %array/at% <idx> value_id in
// <self.patterns[idx].compile_part(local_value_id, local_bindings_id)>
//
// <end fold>
Expand All @@ -686,7 +686,7 @@ impl CompilePart for ArrayPattern {
// <self.patterns[idx].compile_part(local_value_id, local_bindings_id)>
let updated_bindings_let = elem_pat.compile_part(local_value_id, local_bindings_id);

// %array_access% idx value_id
// %array/at% idx value_id
let extracted_value = make::op2(
BinaryOp::ArrayAt,
Term::Var(value_id),
Expand Down Expand Up @@ -786,21 +786,21 @@ impl CompilePart for ArrayPattern {

impl CompilePart for EnumPattern {
fn compile_part(&self, value_id: LocIdent, bindings_id: LocIdent) -> RichTerm {
// %enum_get_tag% value_id == '<self.tag>
// %enum/get_tag% value_id == '<self.tag>
let tag_matches = make::op2(
BinaryOp::Eq,
make::op1(UnaryOp::EnumGetTag, Term::Var(value_id)),
Term::Enum(self.tag),
);

if let Some(pat) = &self.pattern {
// if %enum_is_variant% value_id && %enum_get_tag% value_id == '<self.tag> then
// let value_id = %enum_unwrap_variant% value_id in
// if %enum/is_variant% value_id && %enum/get_tag% value_id == '<self.tag> then
// let value_id = %enum/get_arg% value_id in
// <pattern.compile(value_id, bindings_id)>
// else
// null

// %enum_is_variant% value_id && <tag_matches>
// %enum/is_variant% value_id && <tag_matches>
let if_condition = mk_app!(
make::op1(
UnaryOp::BoolAnd,
Expand All @@ -819,7 +819,7 @@ impl CompilePart for EnumPattern {
Term::Null,
)
} else {
// if %typeof% value_id == 'Enum && !(%enum_is_variant% value_id) && <tag_matches> then
// if %typeof% value_id == 'Enum && !(%enum/is_variant% value_id) && <tag_matches> then
// bindings_id
// else
// null
Expand All @@ -831,7 +831,7 @@ impl CompilePart for EnumPattern {
Term::Enum("Enum".into()),
);

// !(%enum_is_variant% value_id)
// !(%enum/is_variant% value_id)
let is_enum_tag = make::op1(
UnaryOp::BoolNot,
make::op1(UnaryOp::EnumIsVariant, Term::Var(value_id)),
Expand Down
4 changes: 2 additions & 2 deletions core/src/typ.rs
Original file line number Diff line number Diff line change
Expand Up @@ -980,7 +980,7 @@ impl Subcontract for EnumRows {
// x |> match {
// 'foo => x,
// 'bar => x,
// 'Baz variant_arg => 'Baz (%apply_contract% T label_arg variant_arg),
// 'Baz variant_arg => 'Baz (%contract/apply% T label_arg variant_arg),
// _ => $enum_fail l
// }
// ```
Expand All @@ -996,7 +996,7 @@ impl Subcontract for EnumRows {
});

let body = if let Some(ty) = row.typ.as_ref() {
// 'Tag (%apply_contract% T label_arg variant_arg)
// 'Tag (%contract/apply% T label_arg variant_arg)
let arg = mk_app!(
mk_term::op2(
BinaryOp::ContractApply,
Expand Down
17 changes: 9 additions & 8 deletions core/stdlib/internals.ncl
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,17 @@
if polarity == current_polarity then
%unseal% sealing_key value (%blame% label)
else
# [^forall_chng_pol]: Blame assignment for polymorphic contracts
# should take into account the polarity at the point the forall was
# introduced, not the current polarity of the variable occurrence. Indeed,
# forall can never blame in a negative position (relative to the
# [^forall_label_flip_polarity]: Blame assignment for polymorphic
# contracts should take into account the polarity at the point the forall
# was introduced, not the current polarity of the variable occurrence.
# Indeed, forall can never blame in a negative position (relative to the
# forall): the contract is entirely on the callee.
#
# Thus, for correct blame assignment, we want to set the polarity to the
# forall polarity (here `polarity`). Because we only have the `chng_pol`
# primop, and we know that in this branch they are unequal, flipping the
# current polarity will indeed give the original forall's polarity.
# forall polarity (here `polarity`). Because we only have the
# `%label/flip_polarity%` primop, and we know that in this branch they are
# unequal, flipping the current polarity will indeed give the original
# forall's polarity.
%seal% sealing_key (%label/flip_polarity% label) value,

"$forall" = fun sealing_key polarity contract label value =>
Expand Down Expand Up @@ -254,7 +255,7 @@
label
)
else
# See [^forall_chng_pol]
# See [^forall_label_flip_polarity]
%record/seal_tail% sealing_key (%label/flip_polarity% label) acc value,

"$dyn_tail" = fun acc label value => acc & value,
Expand Down
8 changes: 4 additions & 4 deletions notes/fixing-sealing-and-recursive-records.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,14 @@ Final result:
Run of a `%record/map% { foo | Num = 1, bar | PosNat | Even = foo + 1} (fun _key ((+) 1)`

```
App(Op1(record_map, record), function)
Op1(record_map, record); {stack..}
App(Op1(record/map, record), function)
Op1(record/map, record); {stack..}
record ~ RecRecord {..}
RECORD{ foo |pending Num = %1, bar |pending [PosNat,Even] = %2}
%2 := (%apply_contract% Num label %1) + 1
%2 := (%contract/apply% Num label %1) + 1
RECORD{ foo = f (%apply_contract% Num %1), bar = f (%apply_contract% PosNat,Even %2) }
RECORD{ foo = f (%contract/apply% Num %1), bar = f (%contract/apply% PosNat,Even %2) }
# With a call to pending_contract.dualize()
Expand Down

0 comments on commit 090984d

Please sign in to comment.