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

Allow built-in functions to be construct only #1603

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

Conversation

svaarala
Copy link
Owner

@svaarala svaarala commented Jul 10, 2017

With the explicit DUK_HOBJECT_FLAG_CALLABLE it's now possible to create functions that are constructable but not callable as normal functions. This is a prototype branch to see how that would work out in practice for built-in functions. The upside would be that no explicit duk_require_constructor_call() would be required which is smaller (and slightly faster). It would then be possible to expose a public API to create non-constructor and construct-only functions.

Work in progress.

@fatcerberus
Copy link
Contributor

Is duk_require_constructor_call() in any public Duktape releases yet?

@svaarala
Copy link
Owner Author

No, it's an internal function, but it would make sense to expose it regardless of the changes here because it complements duk_is_constructor_call().

@@ -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

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants