Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Do not propagate user-code across Abstract Closure boundaries #382

Merged
merged 1 commit into from
Dec 9, 2021
+54 −4
Merged

Do not propagate user-code across Abstract Closure boundaries #382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/Spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ export default class Spec {

this.autolink();

this.log('Propagating can-call-user-code annotations...');
this.log('Propagating effect annotations...');
this.propagateEffects();
this.log('Linking xrefs...');
this._xrefs.forEach(xref => xref.build());
Expand Down
20 changes: 17 additions & 3 deletions src/Xref.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,22 @@ export default class Xref extends Builder {

canHaveEffect(effectName: string) {
if (!this.isInvocation) return false;
if (this.clause && !this.clause.canHaveEffect(effectName)) {
return false;
if (this.clause) {
// Xrefs nested inside Abstract Closures should not propagate the
// user-code effect, since those are effectively nested functions.
//
// Calls to Abstract Closures that can call user code must be explicitly
// marked as such with <emu-meta effects="user-code">...</emu-meta>.
for (let node = this.node; node.parentElement; node = node.parentElement) {
const parent = node.parentElement;
// This is super hacky. It's checking the output of ecmarkdown.
if (parent.tagName === 'LI' && parent.textContent?.includes('be a new Abstract Closure')) {
return false;
}
}
if (!this.clause.canHaveEffect(effectName)) {
return false;
}
}
if (this.suppressEffects !== null) {
return !this.suppressEffects.includes(effectName);
Expand Down Expand Up @@ -223,7 +237,7 @@ export default class Xref extends Builder {
}
if (this.addEffects !== null) {
if (effects !== null) {
effects.push(...this.addEffects);
effects = effects.concat(...this.addEffects);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a drive-by. Was mutating the AO's existing effects list everytime a particular call site manually added an effect.

} else {
effects = this.addEffects;
}
Expand Down
12 changes: 12 additions & 0 deletions test/baselines/generated-reference/effect-user-code.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,4 +90,16 @@ <h1><span class="secnum">15</span> RenderedMeta()</h1>
<p>The abstract operation RenderedMeta takes no arguments. emu-meta tags with the effects attribute that aren't surrounding what ecmarkup recognizes as invocations are changed into span tags to be rendered. The effects list is prefixed with e- and changed into class names. It performs the following steps when called:</p>
<emu-alg><ol><li>Perform ?&nbsp;<span class="e-user-code"><var>O</var>.[[Call]]()</span>.</li></ol></emu-alg>
</emu-clause>

<emu-clause id="sec-make-abstract-closure" type="abstract operation" aoid="MakeAbstractClosure">
<h1><span class="secnum">16</span> MakeAbstractClosure()</h1>
<p>The abstract operation MakeAbstractClosure takes no arguments. The user-code effect doesn't propagate through Abstract Closure boundaries by recognizing the "be a new Abstract Closure" substring. It performs the following steps when called:</p>
<emu-alg><ol><li>Let <var>closure</var> be a new Abstract Closure that captures nothing and performs the following steps when called:<ol><li><emu-xref aoid="UserCode" id="_ref_12"><a href="#sec-user-code" class="e-user-code">UserCode</a></emu-xref>().</li></ol></li><li>Return <var>closure</var>.</li></ol></emu-alg>
</emu-clause>

<emu-clause id="sec-call-make-abstract-closure" type="abstract operation" aoid="CallMakeAbstractClosure">
<h1><span class="secnum">17</span> CallMakeAbstractClosure()</h1>
<p>The abstract operation CallMakeAbstractClosure takes no arguments. The user-code effect doesn't propagate through Abstract Closure boundaries by recognizing the "be a new Abstract Closure" substring. It performs the following steps when called:</p>
<emu-alg><ol><li><emu-xref aoid="MakeAbstractClosure" id="_ref_13"><a href="#sec-make-abstract-closure">MakeAbstractClosure</a></emu-xref>().</li></ol></emu-alg>
</emu-clause>
</div></body>
24 changes: 24 additions & 0 deletions test/baselines/sources/effect-user-code.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,3 +166,27 @@ <h1>RenderedMeta()</h1>
1. Perform ? <emu-meta effects="user-code">_O_.[[Call]]()</emu-meta>.
</emu-alg>
</emu-clause>

<emu-clause id="sec-make-abstract-closure" type="abstract operation">
<h1>MakeAbstractClosure()</h1>
<dl class="header">
<dt>description</dt>
<dd>The user-code effect doesn't propagate through Abstract Closure boundaries by recognizing the "be a new Abstract Closure" substring.</dd>
</dl>
<emu-alg>
1. Let _closure_ be a new Abstract Closure that captures nothing and performs the following steps when called:
1. UserCode().
1. Return _closure_.
</emu-alg>
</emu-clause>

<emu-clause id="sec-call-make-abstract-closure" type="abstract operation">
<h1>CallMakeAbstractClosure()</h1>
<dl class="header">
<dt>description</dt>
<dd>The user-code effect doesn't propagate through Abstract Closure boundaries by recognizing the "be a new Abstract Closure" substring.</dd>
</dl>
<emu-alg>
1. MakeAbstractClosure().
</emu-alg>
</emu-clause>