String.prototype.concat() #159
Conversation
| if (ecma_is_value_undefined (argument_list_p[arg_index])) | ||
| { | ||
| // FIXME: use ecma_get_string_from_value when it recognizes undefined | ||
| ecma_string_t *string_undef = ecma_new_ecma_string ((unsigned char *)"undefined"); |
There was a problem hiding this comment.
AFAIK: there is already a magic string for this, we should use that.
There was a problem hiding this comment.
Also I think that the ecma_op_to_string correctly returns the "undefined" string if the argument is really undefined. So no need to check the value here.
egavrin
added
ecma builtins
lvidacs
force-pushed
the
string_prototype_concat
branch
from
81a8762 to
4d46c9b
Compare
|
The patch is updated. |
|
|
||
| // check undefined | ||
| try { | ||
| "Check ".concat(x); |
There was a problem hiding this comment.
This does not test what you want, because the access of the x will throw the Reference error before the concat was even called
lvidacs
force-pushed
the
string_prototype_concat
branch
from
4d46c9b to
ca74ae7
Compare
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
There was a problem hiding this comment.
We would need one more test where the arguments of the concat throws an error when the to_string is called.
lvidacs
force-pushed
the
string_prototype_concat
branch
from
ca74ae7 to
4e2a81a
Compare
|
Tests added and the code is fixed, thanks for the feedback. |
| "a".concat(x); | ||
| assert(false); | ||
| } catch (e) { | ||
| print(e instanceof ReferenceError); |
There was a problem hiding this comment.
This should be an assert instead
JerryScript-DCO-1.0-Signed-off-by: Laszlo Vidacs [email protected]
lvidacs
force-pushed
the
string_prototype_concat
branch
from
4e2a81a to
1850cfb
Compare
|
lgtm |
|
Good to me, |
|
Rebase & merged: 72eb14f |
JerryScript-DCO-1.0-Signed-off-by: Laszlo Vidacs [email protected]