-
Notifications
You must be signed in to change notification settings - Fork 10
fixes #25: add HostHasSourceTextAvailable hook #26
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Provided that any exploration of JS-controlled per-function/per-module/per-file censoring explores the web compatibility of returning "[unavailable code]" or similar, this seems fine to me.
Firefox OS (mobile) had something similar IIRC. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!!
<emu-clause id="proposal-sec-hosthassourcetextavailable"> | ||
<h1>HostHasSourceTextAvailable ( _func_ )</h1> | ||
<p>HostHasSourceTextAvailable is an implementation-defined abstract operation that allows host environments to prevent the source text from being provided for a given function.</p> | ||
<p>An implementation of HostHasSourceTextAvailable may complete normally or abruptly. Any abrupt completions will be propagated to its callers. The default implementation of HostHasSourceTextAvailable is to unconditionally return a normal completion with a value of *true*.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure we need the flexibility to return abruptly, but it doesn't hurt anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I figured since toString
can already return abruptly today, it didn't hurt to allow the engine to control that as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allowing abrupt completions doesn't seem useful to me. Function.prototype.toString
is currently spec'ed to either return a string or throw a TypeError. With this change, it either returns a string or throws any value, so the error case is no longer restricted to TypeError.
For example test262 tests will need to be written as:
function f() {}
var errored = false;
var source;
try {
source = f.toString();
} catch {
// HostHasSourceTextAvailable is allowed to return abruptly, unconditionally pass in this case.
errored = true;
}
if (!errored) {
// NATIVE_FUNCTION_RE is defined in test262/harness/nativeFunctionMatcher.js
assert(source === "function f() {}" || NATIVE_FUNCTION_RE.test(source));
}
If we don't allow abrupt completions, the test can be written as:
function f() {}
var source = f.toString();
assert(source === "function f() {}" || NATIVE_FUNCTION_RE.test(source));
Nit: The result must be compared to true
, so HostHasSourceTextAvailable(func) is true, then return ...
instead of HostHasSourceTextAvailable(func), then return ...
. And we should probably require HostHasSourceTextAvailable to return a boolean value.
And I assume not requiring stable results is intentional? So HostHasSourceTextAvailable is allowed to return different results over time even when the same argument is passed to it?
@erights I invite you to voice any concerns you have in this PR. |
Hi @michaelficarra I still don't like the host hook. I think it is a mistake. Most of what it legitimately tries to accomplish is better accomplished with the lexical directive. That said, in a world of scarce attention and "pick your battles" I am not inclined to pick this one. |
@michaelficarra, echoing @erights, I can live with the host hook but I don't know that it is needed at this point. Would it be enough to simply modify the existing specification text, e.g. "If the implementation cannot produce a source code string that meets these criteria then it must return NativeFunction"? This defers definition of mechanisms, such as those in the new censor proposal, while guaranteeing return of a consist string, which addresses the overall goal of this proposal, as I understand it. |
That sounds like just an underspecified version of the host hook, which requires hosts to say "An implementation is unable to produce a source code string under the following circumstances:" instead of "An implementation defines HostHasSourceTextAvailable as:" |
Hi @phoddie I agree with @domenic on this point. The reason I think this whole mechanism is a mistake is its effect on what a correct program can assume. A correct program's correct behavior relies on some underlying platform obeying the contract the correct program correctly relies on. Both the explicit host hook and your suggested text limits these contracts in the same painful way. But the host hook makes the pain explicit, which is better. |
@domenic it is exactly an underspecified version of the host hook, underspecified precisely consistent with the current spec text. As you, @erights, and @michaelficarra are comfortable adding the host hook now, we should move ahead with that. |
@phoddie In that case, can you review and approve this PR? |
@michaelficarra, done. |
If this is sufficient, this was way easier than I thought it would be.
/cc @domenic @phoddie
Fixes #25.