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

add "skip global checks" attribute for AOs #559

Merged
merged 2 commits into from
Nov 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions spec/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ <h1>Structured Headers</h1>
<li><b>effects:</b> A list of "effects" associated with the clause (and transitively with those referencing it), separated by commas with optional whitespace. The only currently-known effect is "user-code", which indicates that the operation or method can evaluate non-implementation code (such as custom getters).</li>
<li><b>for:</b> The type of value to which a clause of type "concrete method" or "internal method" applies.</li>
<li><b>redefinition:</b> If "true", the name of the operation will not automatically link (i.e., it will not automatically be given an aoid).</li>
<li><b>skip global checks:</b> If "true", disables consistency checks for this AO which require knowing every callsite.</li>
</ul>
</li>
</ol>
Expand Down
1 change: 1 addition & 0 deletions src/Biblio.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ export interface AlgorithmBiblioEntry extends BiblioEntryBase {
kind?: AlgorithmType;
signature: null | Signature;
effects: string[];
skipGlobalChecks?: boolean;
/*@internal*/ _node?: Element;
}

Expand Down
8 changes: 8 additions & 0 deletions src/Clause.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export default class Clause extends Builder {
examples: Example[];
readonly effects: string[]; // this is held by identity and mutated by Spec.ts
signature: Signature | null;
skipGlobalChecks: boolean;

constructor(spec: Spec, node: HTMLElement, parent: Clause, number: string) {
super(spec, node);
Expand All @@ -68,6 +69,7 @@ export default class Clause extends Builder {
this.editorNotes = [];
this.examples = [];
this.effects = [];
this.skipGlobalChecks = false;

// namespace is either the entire spec or the parent clause's namespace.
let parentNamespace = spec.namespace;
Expand Down Expand Up @@ -206,6 +208,7 @@ export default class Clause extends Builder {
for: _for,
effects,
redefinition,
skipGlobalChecks,
} = parseStructuredHeaderDl(this.spec, type, dl);

const paras = formatPreamble(
Expand Down Expand Up @@ -236,6 +239,8 @@ export default class Clause extends Builder {
}
}

this.skipGlobalChecks = skipGlobalChecks;

this.effects.push(...effects);
for (const effect of effects) {
if (!this.spec._effectWorklist.has(effect)) {
Expand Down Expand Up @@ -372,6 +377,9 @@ export default class Clause extends Builder {
effects: clause.effects,
_node: clause.node,
};
if (clause.skipGlobalChecks) {
op.skipGlobalChecks = true;
}
if (
signature?.return?.kind === 'union' &&
signature.return.types.some(e => e.kind === 'completion') &&
Expand Down
20 changes: 6 additions & 14 deletions src/Spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,10 @@ export default class Spec {
AOs.filter(e => !isUnused(e.signature!.return!)).map(a => [a.aoid, null])
);
const alwaysAssertedToBeNormal: Map<string, null | 'always asserted normal' | 'top'> = new Map(
AOs.filter(e => e.signature!.return!.kind === 'completion').map(a => [a.aoid, null])
// prettier-ignore
AOs
.filter(e => e.signature!.return!.kind === 'completion' && !e.skipGlobalChecks)
.map(a => [a.aoid, null])
);

// TODO strictly speaking this needs to be done in the namespace of the current algorithm
Expand Down Expand Up @@ -700,18 +703,7 @@ export default class Spec {

const biblioEntry = this.biblio.byAoid(calleeName);
if (biblioEntry == null) {
if (
![
'thisTimeValue',
'thisStringValue',
'thisBigIntValue',
'thisNumberValue',
'thisSymbolValue',
'thisBooleanValue',
'toUppercase',
'toLowercase',
].includes(calleeName)
) {
if (!['toUppercase', 'toLowercase'].includes(calleeName)) {
// TODO make the spec not do this
warn(`could not find definition for ${calleeName}`);
}
Expand Down Expand Up @@ -848,7 +840,7 @@ export default class Spec {
// TODO remove this when https://github.com/tc39/ecma262/issues/2412 is fixed
continue;
}
const message = `every call site of ${aoid} asserts the return value is a normal completion; it should be refactored to not return a completion record at all`;
const message = `every call site of ${aoid} asserts the return value is a normal completion; it should be refactored to not return a completion record at all. if this AO is called in ways ecmarkup cannot analyze, add the "skip global checks" attribute to the header.`;
const ruleId = 'always-asserted-normal';
const biblioEntry = this.biblio.byAoid(aoid)!;
if (biblioEntry._node) {
Expand Down
45 changes: 43 additions & 2 deletions src/header-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -401,11 +401,18 @@ export function parseStructuredHeaderDl(
spec: Spec,
type: string | null,
dl: Element
): { description: Element | null; for: Element | null; effects: string[]; redefinition: boolean } {
): {
description: Element | null;
for: Element | null;
effects: string[];
redefinition: boolean;
skipGlobalChecks: boolean;
} {
let description = null;
let _for = null;
let redefinition: boolean | null = null;
let effects: string[] = [];
let skipGlobalChecks: boolean | null = null;
for (let i = 0; i < dl.children.length; ++i) {
const dt = dl.children[i];
if (dt.tagName !== 'DT') {
Expand Down Expand Up @@ -503,6 +510,34 @@ export function parseStructuredHeaderDl(
}
break;
}
case 'skip global checks': {
if (skipGlobalChecks != null) {
spec.warn({
type: 'node',
ruleId: 'header-format',
message: `duplicate "skip global checks" attribute`,
node: dt,
});
}
const contents = (dd.textContent ?? '').trim();
if (contents === 'true') {
skipGlobalChecks = true;
} else if (contents === 'false') {
skipGlobalChecks = false;
} else {
spec.warn({
type: 'contents',
ruleId: 'header-format',
message: `unknown value for "skip global checks" attribute (expected "true" or "false", got ${JSON.stringify(
contents
)})`,
node: dd,
nodeRelativeLine: 1,
nodeRelativeColumn: 1,
});
}
break;
}
case '': {
spec.warn({
type: 'node',
Expand All @@ -523,7 +558,13 @@ export function parseStructuredHeaderDl(
}
}
}
return { description, for: _for, effects, redefinition: redefinition ?? false };
return {
description,
for: _for,
effects,
redefinition: redefinition ?? false,
skipGlobalChecks: skipGlobalChecks ?? false,
};
}

export function formatPreamble(
Expand Down
30 changes: 29 additions & 1 deletion test/typecheck.js
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ describe('typechecking completions', () => {
ruleId: 'always-asserted-normal',
nodeType: 'emu-clause',
message:
'every call site of ExampleAlg asserts the return value is a normal completion; it should be refactored to not return a completion record at all',
'every call site of ExampleAlg asserts the return value is a normal completion; it should be refactored to not return a completion record at all. if this AO is called in ways ecmarkup cannot analyze, add the "skip global checks" attribute to the header.',
}
);
});
Expand Down Expand Up @@ -673,6 +673,34 @@ describe('typechecking completions', () => {
</emu-alg>
</emu-clause>
`);

await assertLintFree(`
<emu-clause id="example" type="abstract operation">
<h1>
ExampleAlg (): either a normal completion containing Number or an abrupt completion
</h1>
<dl class="header">
<dt>skip global checks</dt>
<dd>true</dd>
</dl>
<emu-alg>
1. Let _foo_ be 0.
1. Return ? _foo_.
</emu-alg>
</emu-clause>

<emu-clause id="example2" type="abstract operation">
<h1>
Example2 ()
</h1>
<dl class="header">
</dl>
<emu-alg>
1. Let _x_ be ! ExampleAlg().
1. Return _x_.
</emu-alg>
</emu-clause>
`);
});
});

Expand Down
Loading