From b86b5eb4e4ec58a363af8a5ca01e26a8d52926ca Mon Sep 17 00:00:00 2001 From: Brian Dickens Date: Wed, 3 May 2017 03:08:18 -0600 Subject: [PATCH] Allow SET/PAD to be used with BLANK on BLOCK! targets R3-Alpha and Red let you write `set [a b] 10`, since the thing you were setting to was not a block, would assume you meant to set all the values to that. BUT since you can set things to blocks, this has a bad characteristic of `set [a b] [10]` being treated differently, which can bite you if you `set [a b] value` for some generic value. Ren-C does not carry forward the behavior, rather it allows only one case...to pass in a blank value with /PAD and handle that blank as if it were an empty block, return blanks. This permits things like `if set [a b] find data 'whatever [...]` to do a blank propagation. This is to facilitate the only known client of the behavior, which was the module code. !!! Should there be a SET/ONLY to allow for `set/only [a b] [10]` such that a and b would be set to 10? Adding more complexity to a primitive like SET is not very desirable, and you can do this other ways in usermode if it's what you really want. --- src/core/n-data.c | 38 +++++++++++++++++++++++++++++++------- src/mezz/sys-load.r | 2 +- tests/context/set.test.reb | 11 +++++++++++ 3 files changed, 43 insertions(+), 8 deletions(-) diff --git a/src/core/n-data.c b/src/core/n-data.c index 634453d298..964b8ba0ff 100755 --- a/src/core/n-data.c +++ b/src/core/n-data.c @@ -858,17 +858,41 @@ REBNATIVE(set) REBSPC *target_specifier; if (IS_BLOCK(ARG(target))) { - if (NOT(IS_BLOCK(ARG(value)))) + // + // R3-Alpha and Red let you write `set [a b] 10`, since the thing + // you were setting to was not a block, would assume you meant to set + // all the values to that. BUT since you can set things to blocks, + // this has a bad characteristic of `set [a b] [10]` being treated + // differently, which can bite you if you `set [a b] value` for some + // generic value. + // + // Ren-C does not carry forward the behavior, rather it allows only + // one case...to pass in a blank value with /PAD and handle that blank + // as if it were an empty block, return blanks. This permits things + // like `if set [a b] find data 'whatever [...]` to do a blank + // propagation. + // + // !!! Should there be a SET/ONLY to allow for `set/only [a b] [10]` + // such that a and b would be set to 10? Seems to add complexity for + // a case that is not that compelling. + // + if (IS_BLANK(ARG(value)) && REF(pad)) { + value = ARR_HEAD(EMPTY_ARRAY); // don't change ARG(value) + value_specifier = SPECIFIED; + } + else if (IS_BLOCK(ARG(value))) { + // + // There is no need to check values for voidness in this case, + // since arrays cannot contain voids. + // + value = VAL_ARRAY_AT(ARG(value)); + value_specifier = VAL_SPECIFIER(ARG(value)); + } + else fail (ARG(value)); // value must be a block if setting a block target = VAL_ARRAY_AT(ARG(target)); target_specifier = VAL_SPECIFIER(ARG(target)); - - // There is no need to check values for voidness in this case, since - // arrays cannot contain voids. - // - value = VAL_ARRAY_AT(ARG(value)); - value_specifier = VAL_SPECIFIER(ARG(value)); } else { // Use the fact that D_CELL is implicitly terminated so that the diff --git a/src/mezz/sys-load.r b/src/mezz/sys-load.r index 35041371cd..f3d858aa40 100644 --- a/src/mezz/sys-load.r +++ b/src/mezz/sys-load.r @@ -783,7 +783,7 @@ load-module: function [ ; set to false later if existing module is used override?: not no-lib - set [name0: mod0: sum0:] pos: find/skip system/modules name 3 + set/pad [name0: mod0: sum0:] pos: find/skip system/modules name 3 ] [ ; Get existing module's info case/all [ diff --git a/tests/context/set.test.reb b/tests/context/set.test.reb index c40b3b3fdf..e09484c33c 100644 --- a/tests/context/set.test.reb +++ b/tests/context/set.test.reb @@ -6,3 +6,14 @@ [x: has [a: 1 b: 2] all [error? try [set x reduce [3 ()]] x/a = 1]] ; set [:get-word] [word] [a: 1 b: _ set [b] [a] b = 'a] + +; Behavior in R3-Alpha allowed `set [a b] 10` to set both, but created a +; bad variance with `set [a b] [10]`. Rather than complicate SET with /ONLY, +; the behavior going forward is just to allow blanking with pad, so that +; `set/pad [a b] blank` gives the same effect as `set/pad [a b] []`, except +; returning blank instead of the empty block. +[ + a: 10 + b: 20 + all? [blank = set/pad [a b] blank | blank? a | blank? b] +]