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.

Sign up for GitHub

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

Allow built-in functions to be construct only #1603

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Allow built-in functions to be construct only #1603

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
65a221d
Genbuiltins 'callable' improvements
svaarala Jul 10, 2017
a759c79
Allow built-ins to be construct only
svaarala Jul 10, 2017
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
30 changes: 15 additions & 15 deletions src-input/builtins.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3015,7 +3015,7 @@ objects:
internal_prototype: bi_function_prototype
# no external prototype
native: duk_bi_proxy_constructor
callable: true
callable: false
constructable: true
es6: true
bidx: true
Expand Down Expand Up @@ -3329,7 +3329,7 @@ objects:
internal_prototype: bi_function_prototype
varargs: false
native: duk_bi_arraybuffer_constructor
callable: true
callable: false
constructable: true
typedarray: true
es6: true
Expand Down Expand Up @@ -3416,7 +3416,7 @@ objects:
internal_prototype: bi_function_prototype
varargs: false
native: duk_bi_dataview_constructor
callable: true
callable: false
constructable: true
typedarray: true
es6: true
Expand Down Expand Up @@ -3737,7 +3737,7 @@ objects:
internal_prototype: bi_function_prototype
varargs: false
native: duk_bi_typedarray_constructor
callable: true
callable: false
constructable: false
nargs: 0
magic: 0
Expand Down Expand Up @@ -3852,7 +3852,7 @@ objects:
internal_prototype: bi_typedarray_constructor
varargs: false
native: duk_bi_typedarray_constructor
callable: true
callable: false
constructable: true
magic:
type: typedarray_constructor
Expand Down Expand Up @@ -3920,7 +3920,7 @@ objects:
internal_prototype: bi_typedarray_constructor
varargs: false
native: duk_bi_typedarray_constructor
callable: true
callable: false
constructable: true
magic:
type: typedarray_constructor
Expand Down Expand Up @@ -4009,7 +4009,7 @@ objects:
internal_prototype: bi_typedarray_constructor
varargs: false
native: duk_bi_typedarray_constructor
callable: true
callable: false
constructable: true
magic:
type: typedarray_constructor
Expand Down Expand Up @@ -4077,7 +4077,7 @@ objects:
internal_prototype: bi_typedarray_constructor
varargs: false
native: duk_bi_typedarray_constructor
callable: true
callable: false
constructable: true
magic:
type: typedarray_constructor
Expand Down Expand Up @@ -4145,7 +4145,7 @@ objects:
internal_prototype: bi_typedarray_constructor
varargs: false
native: duk_bi_typedarray_constructor
callable: true
callable: false
constructable: true
magic:
type: typedarray_constructor
Expand Down Expand Up @@ -4213,7 +4213,7 @@ objects:
internal_prototype: bi_typedarray_constructor
varargs: false
native: duk_bi_typedarray_constructor
callable: true
callable: false
constructable: true
magic:
type: typedarray_constructor
Expand Down Expand Up @@ -4281,7 +4281,7 @@ objects:
internal_prototype: bi_typedarray_constructor
varargs: false
native: duk_bi_typedarray_constructor
callable: true
callable: false
constructable: true
magic:
type: typedarray_constructor
Expand Down Expand Up @@ -4349,7 +4349,7 @@ objects:
internal_prototype: bi_typedarray_constructor
varargs: false
native: duk_bi_typedarray_constructor
callable: true
callable: false
constructable: true
magic:
type: typedarray_constructor
Expand Down Expand Up @@ -4417,7 +4417,7 @@ objects:
internal_prototype: bi_typedarray_constructor
varargs: false
native: duk_bi_typedarray_constructor
callable: true
callable: false
constructable: true
magic:
type: typedarray_constructor
Expand Down Expand Up @@ -5100,7 +5100,7 @@ objects:
internal_prototype: bi_function_prototype
nargs: 0
native: duk_bi_textencoder_constructor
callable: true
callable: false
constructable: true
bidx: true
encoding_api: true
Expand Down Expand Up @@ -5158,7 +5158,7 @@ objects:
internal_prototype: bi_function_prototype
nargs: 2
native: duk_bi_textdecoder_constructor
callable: true
callable: false
constructable: true
bidx: true
encoding_api: true
Expand Down
8 changes: 4 additions & 4 deletions src-input/duk_bi_buffer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,7 @@ DUK_INTERNAL duk_ret_t duk_bi_arraybuffer_constructor(duk_context *ctx) {
thr = (duk_hthread *) ctx;
DUK_UNREF(thr);

duk_require_constructor_call(ctx);
DUK_ASSERT(!duk_is_constructor_call(ctx));

len = duk_to_int(ctx, 0);
if (len < 0) {
Expand Down Expand Up @@ -722,7 +722,7 @@ DUK_INTERNAL duk_ret_t duk_bi_typedarray_constructor(duk_context *ctx) {
* buffer functions.
*/

duk_require_constructor_call(ctx);
DUK_ASSERT(!duk_is_constructor_call(ctx));

/* We could fit built-in index into magic but that'd make the magic
* number dependent on built-in numbering (genbuiltins.py doesn't
Expand Down Expand Up @@ -1061,7 +1061,7 @@ DUK_INTERNAL duk_ret_t duk_bi_typedarray_constructor(duk_context *ctx) {
* buffer functions.
*/

duk_require_constructor_call(ctx);
DUK_ASSERT(!duk_is_constructor_call(ctx));

elem_length_signed = duk_require_int(ctx, 0);
if (elem_length_signed < 0) {
Expand All @@ -1086,7 +1086,7 @@ DUK_INTERNAL duk_ret_t duk_bi_dataview_constructor(duk_context *ctx) {
duk_uint_t offset;
duk_uint_t length;

duk_require_constructor_call(ctx);
DUK_ASSERT(!duk_is_constructor_call(ctx));

h_bufarg = duk__require_bufobj_value(ctx, 0);
DUK_ASSERT(h_bufarg != NULL);
Expand Down
5 changes: 3 additions & 2 deletions src-input/duk_bi_encoding.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,8 @@ DUK_INTERNAL duk_ret_t duk_bi_textencoder_constructor(duk_context *ctx) {
* does nothing on purpose.
*/

duk_require_constructor_call(ctx);
DUK_UNREF(ctx);
DUK_ASSERT(duk_is_constructor_call(ctx));
return 0;
}

Expand Down Expand Up @@ -441,7 +442,7 @@ DUK_INTERNAL duk_ret_t duk_bi_textdecoder_constructor(duk_context *ctx) {
duk_bool_t ignore_bom = 0;

DUK_ASSERT_TOP(ctx, 2);
duk_require_constructor_call(ctx);
DUK_ASSERT(duk_is_constructor_call(ctx));
if (!duk_is_undefined(ctx, 0)) {
/* XXX: For now ignore 'label' (encoding identifier). */
duk_to_string(ctx, 0);
Expand Down
1 change: 1 addition & 0 deletions src-input/duk_bi_json.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2104,6 +2104,7 @@ DUK_LOCAL duk_bool_t duk__enc_value(duk_json_enc_ctx *js_ctx, duk_idx_t idx_hold
* value is NOT looked up like for e.g. String objects).
*/
DUK_ASSERT(h != NULL);
/* FIXME: grep all CALLABLE checks and see if they should be CALLABLE || CONSTRUCTABLE */
if (DUK_HOBJECT_IS_CALLABLE(h)) {
#if defined(DUK_USE_JX) || defined(DUK_USE_JC)
if (js_ctx->flags & (DUK_JSON_FLAG_EXT_CUSTOM |
Expand Down
2 changes: 1 addition & 1 deletion src-input/duk_bi_proxy.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ DUK_INTERNAL void duk_proxy_ownkeys_postprocess(duk_context *ctx, duk_hobject *h
DUK_INTERNAL duk_ret_t duk_bi_proxy_constructor(duk_context *ctx) {
DUK_ASSERT_TOP(ctx, 2); /* [ target handler ] */

duk_require_constructor_call(ctx);
DUK_ASSERT(duk_is_constructor_call(ctx));
duk_push_proxy(ctx); /* [ target handler ] -> [ proxy ] */
return 1; /* replacement */
}
Expand Down
6 changes: 6 additions & 0 deletions src-input/duk_hobject.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@
* inspection code.
*/

/* FIXME: ES spec: all construct-only functions have [[Call]] and [[Construct]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand this. I was under the impression that [[Call]] and [[Construct]] are specification devices, so what does it mean for compliance, in practical terms, to say a function "doesn't have [[Call]]"?

Copy link
Owner Author

@svaarala svaarala Jul 10, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was just a cryptic note to myself so maybe not very readable ;)

In ES all functions have [[Call]], and constructable functions also have [[Construct]]. Functions that are construct-only in the specification reject non-constructor calls specifically by checking for it. All of this is the conceptual specification view.

Right now in this pull the CALLABLE flag behaves differently. It's not set for construct-only functions so that attempts to call them as normal functions will get automatically rejected without any explicit check in the function. So, CALLABLE <!=> [[Call]]. This affects a bunch of checks inside; when the code is supposed to check for existence of [[Call]], the internals needs to check for CALLABLE | CONSTRUCTABLE to get the same effect.

This may be confusing, which is why the FIXME.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understood, I was just trying to figure what it meant in practice--since if there's no observable difference in behavior then it doesn't matter for compliance. But I guess in this case the difference would be that the thrown error would not have the function in its stack trace, while it would in a compliant engine.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would just be an internal difference - the main concern being that DUK_HOBJECT_IS_CALLABLE(x) would be different than "has [[Call]]". At present they are the same in master. This might be error prone when maintaining code unless the naming was changed to make the distinction clearer.

The stack trace behavior would be different from current behavior, but stack traces are not compliancy related.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I see. I was just trying to figure out what needed to be "fixed" (since it's a FIXME). As long as you know the answer to that, I'm satisfied. :)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hehe, yes. I don't keep work in progress branches in a well commented state btw. That's why they're work in progress :)

* so the modeling is not 1:1. Maybe rename so that _CALLABLE means either one?
* _CONSTRUCTABLE..
*/

/* XXX: some flags are object subtype specific (e.g. common to all function
* subtypes, duk_harray, etc) and could be reused for different subtypes.
*/
Expand Down Expand Up @@ -187,6 +192,7 @@
DUK_HOBJECT_FLAG_NATFUNC)

#define DUK_HOBJECT_IS_CALLABLE(h) DUK_HOBJECT_HAS_CALLABLE((h))
#define DUK_HOBJECT_IS_CONSTRUCTABLE(h) DUK_HOBJECT_HAS_CONSTRUCTABLE((h))

/* Object has any exotic behavior(s). */
#define DUK_HOBJECT_EXOTIC_BEHAVIOR_FLAGS (DUK_HOBJECT_FLAG_EXOTIC_ARRAY | \
Expand Down
8 changes: 7 additions & 1 deletion src-input/duk_hthread_builtins.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,8 +279,14 @@ DUK_INTERNAL void duk_hthread_create_builtin_objects(duk_hthread *thr) {

/* Almost all global level Function objects are constructable
* but not all: Function.prototype is a non-constructable,
* callable Function.
* callable Function. Some built-ins are only constructable,
* not callable as normal functions, e.g. Uint8Array constructor.
*/
if (duk_bd_decode_flag(bd)) {
DUK_ASSERT(DUK_HOBJECT_HAS_CALLABLE(h));
} else {
DUK_HOBJECT_CLEAR_CALLABLE(h);
}
if (duk_bd_decode_flag(bd)) {
DUK_ASSERT(DUK_HOBJECT_HAS_CONSTRUCTABLE(h));
} else {
Expand Down
26 changes: 18 additions & 8 deletions src-input/duk_js_call.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1204,12 +1204,19 @@ DUK_LOCAL duk_hobject *duk__resolve_target_func_and_this_binding(duk_context *ct
func = DUK_TVAL_GET_OBJECT(tv_func);

if (*call_flags & DUK_CALL_FLAG_CONSTRUCT) {
if (DUK_UNLIKELY(!DUK_HOBJECT_HAS_CONSTRUCTABLE(func))) {
if (DUK_UNLIKELY(!DUK_HOBJECT_IS_CONSTRUCTABLE(func))) {
goto not_constructable;
}
} else {
if (DUK_UNLIKELY(!DUK_HOBJECT_IS_CALLABLE(func))) {
goto not_callable;
if (DUK_HOBJECT_IS_CONSTRUCTABLE(func)) {
/* Friendly error if calling a construct-only
* function using a normal call.
*/
goto constructor_not_callable;
} else {
goto not_callable;
}
}
}

Expand Down Expand Up @@ -1255,8 +1262,8 @@ DUK_LOCAL duk_hobject *duk__resolve_target_func_and_this_binding(duk_context *ct
#endif
{
DUK_ASSERT(DUK_HOBJECT_IS_NATFUNC(func));
DUK_ASSERT(DUK_HOBJECT_HAS_CALLABLE(func));
DUK_ASSERT(!DUK_HOBJECT_HAS_CONSTRUCTABLE(func));
DUK_ASSERT(DUK_HOBJECT_IS_CALLABLE(func));
DUK_ASSERT(!DUK_HOBJECT_IS_CONSTRUCTABLE(func));
/* Constructable check already done above. */

if (duk__handle_specialfuncs_for_call(thr, idx_func, func, call_flags, first) != 0) {
Expand Down Expand Up @@ -1312,20 +1319,23 @@ DUK_LOCAL duk_hobject *duk__resolve_target_func_and_this_binding(duk_context *ct
DUK_ASSERT(func == NULL || !DUK_HOBJECT_HAS_BOUNDFUNC(func));
DUK_ASSERT(func == NULL || (DUK_HOBJECT_IS_COMPFUNC(func) ||
DUK_HOBJECT_IS_NATFUNC(func)));
DUK_ASSERT(func == NULL || (DUK_HOBJECT_HAS_CONSTRUCTABLE(func) ||
DUK_ASSERT(func == NULL || (DUK_HOBJECT_IS_CONSTRUCTABLE(func) ||
(*call_flags & DUK_CALL_FLAG_CONSTRUCT) == 0));
}
#endif

return func;

constructor_not_callable:
DUK_ERROR_TYPE((duk_hthread *) ctx, DUK_STR_CONSTRUCT_ONLY);
return NULL; /* never executed */

not_callable:
DUK_ASSERT(tv_func != NULL);
#if defined(DUK_USE_VERBOSE_ERRORS)
#if defined(DUK_USE_PARANOID_ERRORS)
DUK_ERROR_FMT1(thr, DUK_ERR_TYPE_ERROR, "%s not callable", duk_get_type_name(ctx, idx_func));
#else
DUK_ERROR_FMT1(thr, DUK_ERR_TYPE_ERROR, "%s not callable", duk_push_string_tval_readable(ctx, tv_func));
DUK_ERROR_FMT1(thr, DUK_ERR_TYPE_ERROR, "%s not callable", duk_push_string_readable(ctx, idx_func));
#endif
#else
DUK_ERROR_TYPE(thr, DUK_STR_NOT_CALLABLE);
Expand All @@ -1338,7 +1348,7 @@ DUK_LOCAL duk_hobject *duk__resolve_target_func_and_this_binding(duk_context *ct
#if defined(DUK_USE_PARANOID_ERRORS)
DUK_ERROR_FMT1(thr, DUK_ERR_TYPE_ERROR, "%s not constructable", duk_get_type_name(ctx, idx_func));
#else
DUK_ERROR_FMT1(thr, DUK_ERR_TYPE_ERROR, "%s not constructable", duk_push_string_tval_readable(ctx, tv_func));
DUK_ERROR_FMT1(thr, DUK_ERR_TYPE_ERROR, "%s not constructable", duk_push_string_readable(ctx, idx_func));
#endif
#else
DUK_ERROR_TYPE(thr, DUK_STR_NOT_CONSTRUCTABLE);
Expand Down
24 changes: 11 additions & 13 deletions src-input/duk_js_ops.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1044,20 +1044,17 @@ DUK_INTERNAL duk_bool_t duk_js_instanceof(duk_hthread *thr, duk_tval *tv_x, duk_
/*
* For bound objects, [[HasInstance]] just calls the target function
* [[HasInstance]]. If that is again a bound object, repeat until
* we find a non-bound Function object.
* we find a non-bound Function object. The bound function chain is
* now "collapsed" so there can be only one bound function in the chain.
*
* The bound function chain is now "collapsed" so there can be only
* one bound function in the chain.
* Of native Ecmascript objects, only Function instances have a
* [[HasInstance]] internal property. Custom objects might also have
* it, but not in current implementation.
*
* XXX: add a separate flag, DUK_HOBJECT_FLAG_ALLOW_INSTANCEOF?
*/

if (!DUK_HOBJECT_IS_CALLABLE(func)) {
/*
* Note: of native Ecmascript objects, only Function instances
* have a [[HasInstance]] internal property. Custom objects might
* also have it, but not in current implementation.
*
* XXX: add a separate flag, DUK_HOBJECT_FLAG_ALLOW_INSTANCEOF?
*/
if (!(DUK_HOBJECT_IS_CALLABLE(func) || DUK_HOBJECT_IS_CONSTRUCTABLE(func))) {
DUK_ERROR_TYPE(thr, "invalid instanceof rval");
}

Expand All @@ -1070,7 +1067,7 @@ DUK_INTERNAL duk_bool_t duk_js_instanceof(duk_hthread *thr, duk_tval *tv_x, duk_
* functions whose target is not proper.
*/
DUK_ASSERT(func != NULL);
DUK_ASSERT(DUK_HOBJECT_IS_CALLABLE(func));
DUK_ASSERT(DUK_HOBJECT_IS_CALLABLE(func) || DUK_HOBJECT_IS_CONSTRUCTABLE(func));
}

/*
Expand All @@ -1082,6 +1079,7 @@ DUK_INTERNAL duk_bool_t duk_js_instanceof(duk_hthread *thr, duk_tval *tv_x, duk_
DUK_ASSERT(func != NULL);
DUK_ASSERT(!DUK_HOBJECT_HAS_BOUNDFUNC(func));
DUK_ASSERT(DUK_HOBJECT_IS_CALLABLE(func));
DUK_ASSERT(DUK_HOBJECT_IS_CALLABLE(func) || DUK_HOBJECT_IS_CONSTRUCTABLE(func));

/* [ ... lval rval(func) ] */

Expand Down Expand Up @@ -1267,7 +1265,7 @@ DUK_INTERNAL duk_small_uint_t duk_js_typeof_stridx(duk_tval *tv_x) {
case DUK_TAG_OBJECT: {
duk_hobject *obj = DUK_TVAL_GET_OBJECT(tv_x);
DUK_ASSERT(obj != NULL);
if (DUK_HOBJECT_IS_CALLABLE(obj)) {
if (DUK_HOBJECT_IS_CALLABLE(obj) || DUK_HOBJECT_IS_CONSTRUCTABLE(obj)) {
stridx = DUK_STRIDX_LC_FUNCTION;
} else {
stridx = DUK_STRIDX_LC_OBJECT;
Expand Down