Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable recursive priorities #1600

Merged
merged 1 commit into from
Sep 13, 2023
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
20 changes: 12 additions & 8 deletions core/src/parser/grammar.lalrpop
Original file line number Diff line number Diff line change
Expand Up @@ -158,14 +158,18 @@ SimpleFieldAnnotAtom<TypeRule>: FieldMetadata = {
// rule).
FieldAnnotAtom<TypeRule>: FieldExtAnnot = {
<SimpleFieldAnnotAtom<TypeRule>> => <>.into(),
"|" "rec" "force" => FieldExtAnnot {
rec_force: true,
..Default::default()
},
"|" "rec" "default" => FieldExtAnnot {
rec_default: true,
..Default::default()
},
// Recursive priorities are disabled as of 1.2.0. Their semantics is non trivial
// to adapt to RFC005 that landed in 1.0.0, so they are currently on hold. If we
// drop them altogether, we'll have to clean the corresponding code floating
// around (not only in the parser, but in the internals module, etc.)
// "|" "rec" "force" => FieldExtAnnot {
// rec_force: true,
// ..Default::default()
// },
// "|" "rec" "default" => FieldExtAnnot {
// rec_default: true,
// ..Default::default()
// },
}

// An annotation, with possibly many annotations chained.
Expand Down
19 changes: 15 additions & 4 deletions core/tests/integration/pass/merging/multiple_overrides.ncl
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,22 @@ let {check, ..} = import "../lib/assert.ncl" in
| default = "",
snd_data = "snd",
},
final_override | rec force = {
fst_data = "override",
snd_data = "override",
common.final = fst_data ++ "_" ++ snd_data,
# Recursive priorities are currently on hold since the implementation of
# RFC005, which changes their semantics. In the meantime, this test has been
# rewritten temporarily with the expected result of the original "rec force"
# instead
#
# final_override | rec force = {
# fst_data = "override",
# snd_data = "override",
# common.final = fst_data ++ "_" ++ snd_data,
# },
final_override = {
fst_data | force = "override",
snd_data | force = "override",
common.final | force = fst_data ++ "_" ++ snd_data,
},

} in
[
parent.fst_data & parent.snd_data == {
Expand Down
44 changes: 22 additions & 22 deletions core/tests/integration/pass/merging/priorities.ncl
Original file line number Diff line number Diff line change
Expand Up @@ -42,29 +42,31 @@ let {Assert, check, ..} = import "../lib/assert.ncl" in
d = 1,
} | Assert,

# TODO: restore (or not?). The previous behavior is harder to simulate after
# RFC005.
# {foo | rec default = 1} & {foo = 2} == {foo = 2} | Assert,
# {foo | rec force = 1} & {foo = 2} == {foo = 1} | Assert,
{val | rec default = {foo = 1}} & {val.foo = 2} == {val.foo = 2} | Assert,
{val | rec force = {foo = 1}} & {val.foo = 2} == {val.foo = 1} | Assert,
# All the following commented out tests are related to recursive priorities,
# which are on hold since RFC005 (and were disabled in 1.2.0). We need to
# rethink their semantics before proceeding further.

#{foo | rec default = 1} & {foo = 2} == {foo = 2} | Assert,
#{foo | rec force = 1} & {foo = 2} == {foo = 1} | Assert,
#{val | rec default = {foo = 1}} & {val.foo = 2} == {val.foo = 2} | Assert,
#{val | rec force = {foo = 1}} & {val.foo = 2} == {val.foo = 1} | Assert,

# Pushed priorities should only modifies fields without explicitly set priorities
{val | rec force = {foo | priority -1 = 1}} & {val.foo = 2} == {val.foo = 2} | Assert,
{val | rec force = {foo | default = 1}} & {val.foo = 2} == {val.foo = 2} | Assert,
{val | rec default = {foo | priority 1 = 1}} & {val.foo = 2} == {val.foo = 1} | Assert,
{val | rec default = {foo | force = 1}} & {val.foo = 2} == {val.foo = 1} | Assert,
#{val | rec force = {foo | priority -1 = 1}} & {val.foo = 2} == {val.foo = 2} | Assert,
#{val | rec force = {foo | default = 1}} & {val.foo = 2} == {val.foo = 2} | Assert,
#{val | rec default = {foo | priority 1 = 1}} & {val.foo = 2} == {val.foo = 1} | Assert,
#{val | rec default = {foo | force = 1}} & {val.foo = 2} == {val.foo = 1} | Assert,

let x = {
foo | force = "old",
bar = {
baz = "old",
baz' = "old"
} |> std.record.map (fun _name value => value)
} in
{val | rec default = x} & {val = {foo = "new", bar.baz = "new"}}
== { val = {foo = "old", bar = {baz = "new", baz' = "old"}}}
| Assert,
#let x = {
# foo | force = "old",
# bar = {
# baz = "old",
# baz' = "old"
# } |> std.record.map (fun _name value => value)
#} in
#{val | rec default = x} & {val = {foo = "new", bar.baz = "new"}}
# == { val = {foo = "old", bar = {baz = "new", baz' = "old"}}}
# | Assert,

# Interaction of recursive overriding and recursive priorities
#
Expand All @@ -74,8 +76,6 @@ let {Assert, check, ..} = import "../lib/assert.ncl" in
# (without `rec force` but with `force` directly) works.
# Maybe some field update issue?
#
# TODO: restore and understand what's going on
#
# let context = 1 in
# let x = {
# foo = context + 2 + bar,
Expand Down