Skip to content
Permalink
Browse files

fix(ivy): avoid infinite recursion when evaluation source files (#33772)

When ngtsc comes across a source file during partial evaluation, it
would determine all exported symbols from that module and evaluate their
values greedily. This greedy evaluation strategy introduces unnecessary
work and can fall into infinite recursion when the evaluation result of
an exported expression would circularly depend on the source file. This
would primarily occur in CommonJS code, where the `exports` variable can
be used to refer to an exported variable. This variable would be
resolved to the source file itself, thereby greedily evaluating all
exported symbols and thus ending up evaluating the `exports` variable
again. This variable would be resolved to the source file itself,
thereby greedily evaluating all exported symbols and thus ending u
evaluating the `exports` variable again. This variable would be
resolved to the source file itself, thereby greedily evaluating all
exported symbols and thus ending up evaluating the `exports` variable
again. This variable would be resolved to the source file itself,
thereby greedily evaluating all exported symbols and thus ending up
evaluating the `exports` variable again. This went on for some time
until all stack frames were exhausted.

This commit introduces a `ResolvedModule` that delays the evaluation of
its exports until they are actually requested. This avoids the circular
dependency when evaluating `exports`, thereby fixing the issue.

Fix #33734

PR Close #33772
  • Loading branch information
JoostK authored and alxhub committed Nov 12, 2019
1 parent 94257ac commit b12fde4ceb885d0579feffb2b17791ad605421cc
@@ -16,7 +16,7 @@ import {isDeclaration} from '../../util/src/typescript';
import {ArrayConcatBuiltinFn, ArraySliceBuiltinFn} from './builtin';
import {DynamicValue} from './dynamic';
import {DependencyTracker, ForeignFunctionResolver} from './interface';
import {BuiltinFn, EnumValue, ResolvedValue, ResolvedValueArray, ResolvedValueMap} from './result';
import {BuiltinFn, EnumValue, ResolvedModule, ResolvedValue, ResolvedValueArray, ResolvedValueMap} from './result';
import {evaluateTsHelperInline} from './ts_helpers';


@@ -183,11 +183,14 @@ export class StaticInterpreter {
const spread = this.visitExpression(property.expression, context);
if (spread instanceof DynamicValue) {
return DynamicValue.fromDynamicInput(node, spread);
} else if (!(spread instanceof Map)) {
} else if (spread instanceof Map) {
spread.forEach((value, key) => map.set(key, value));
} else if (spread instanceof ResolvedModule) {
spread.getExports().forEach((value, key) => map.set(key, value));
} else {
return DynamicValue.fromDynamicInput(
node, DynamicValue.fromInvalidExpressionType(property, spread));
}
spread.forEach((value, key) => map.set(key, value));
} else {
return DynamicValue.fromUnknown(node);
}
@@ -323,19 +326,18 @@ export class StaticInterpreter {
if (declarations === null) {
return DynamicValue.fromUnknown(node);
}
const map = new Map<string, ResolvedValue>();
declarations.forEach((decl, name) => {

return new ResolvedModule(declarations, decl => {
const declContext = {
...context, ...joinModuleContext(context, node, decl),
};

// Visit both concrete and inline declarations.
// TODO(alxhub): remove cast once TS is upgraded in g3.
const value = decl.node !== null ?
return decl.node !== null ?
this.visitDeclaration(decl.node, declContext) :
this.visitExpression((decl as InlineDeclaration).expression, declContext);
map.set(name, value);
});
return map;
}

private accessHelper(
@@ -348,6 +350,8 @@ export class StaticInterpreter {
} else {
return undefined;
}
} else if (lhs instanceof ResolvedModule) {
return lhs.getExport(strIndex);
} else if (Array.isArray(lhs)) {
if (rhs === 'length') {
return lhs.length;
@@ -9,6 +9,7 @@
import * as ts from 'typescript';

import {Reference} from '../../imports';
import {Declaration} from '../../reflection';

import {DynamicValue} from './dynamic';

@@ -21,23 +22,47 @@ import {DynamicValue} from './dynamic';
* available statically.
*/
export type ResolvedValue = number | boolean | string | null | undefined | Reference | EnumValue |
ResolvedValueArray | ResolvedValueMap | BuiltinFn | DynamicValue<unknown>;
ResolvedValueArray | ResolvedValueMap | ResolvedModule | BuiltinFn | DynamicValue<unknown>;

/**
* An array of `ResolvedValue`s.
*
* This is a reified type to allow the circular reference of `ResolvedValue` -> `ResolvedValueArray`
* ->
* `ResolvedValue`.
* -> `ResolvedValue`.
*/
export interface ResolvedValueArray extends Array<ResolvedValue> {}

/**
* A map of strings to `ResolvedValue`s.
*
* This is a reified type to allow the circular reference of `ResolvedValue` -> `ResolvedValueMap` ->
* `ResolvedValue`.
*/ export interface ResolvedValueMap extends Map<string, ResolvedValue> {}
* This is a reified type to allow the circular reference of `ResolvedValue` -> `ResolvedValueMap`
* -> `ResolvedValue`.
*/
export interface ResolvedValueMap extends Map<string, ResolvedValue> {}

/**
* A collection of publicly exported declarations from a module. Each declaration is evaluated
* lazily upon request.
*/
export class ResolvedModule {
constructor(
private exports: Map<string, Declaration>,
private evaluate: (decl: Declaration) => ResolvedValue) {}

getExport(name: string): ResolvedValue {
if (!this.exports.has(name)) {
return undefined;
}

return this.evaluate(this.exports.get(name) !);
}

getExports(): ResolvedValueMap {
const map = new Map<string, ResolvedValue>();
this.exports.forEach((decl, name) => { map.set(name, this.evaluate(decl)); });
return map;
}
}

/**
* A value member of an enumeration.
@@ -389,6 +389,38 @@ runInEachFileSystem(() => {
});
});

it('module spread works', () => {
const map = evaluate<Map<string, number>>(
`import * as mod from './module'; const c = {...mod, c: 3};`, 'c', [
{name: _('/module.ts'), contents: `export const a = 1; export const b = 2;`},
]);

const obj: {[key: string]: number} = {};
map.forEach((value, key) => obj[key] = value);
expect(obj).toEqual({
a: 1,
b: 2,
c: 3,
});
});

it('evaluates module exports lazily to avoid infinite recursion', () => {
const value = evaluate(`import * as mod1 from './mod1';`, 'mod1.primary', [
{
name: _('/mod1.ts'),
contents: `
import * as mod2 from './mod2';
export const primary = mod2.indirection;
export const secondary = 2;`
},
{
name: _('/mod2.ts'),
contents: `import * as mod1 from './mod1'; export const indirection = mod1.secondary;`
},
]);
expect(value).toEqual(2);
});

it('indirected-via-object function call works', () => {
expect(evaluate(
`

0 comments on commit b12fde4

Please sign in to comment.
You can’t perform that action at this time.