fix: parameter variable infinite recursion error (#5500)

* fix: parameter variable infinite recursion error Ensure before hasEffects of function body (triggered by function call), the arguments are first provided * update test
rollup · Apr 29, 2024 · 571929f · 571929f
1 parent 9135249
commit 571929f
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 23 deletions.
diff --git a/src/ast/nodes/NewExpression.ts b/src/ast/nodes/NewExpression.ts
@@ -19,20 +19,17 @@ export default class NewExpression extends NodeBase {
 	declare annotationPure?: boolean;
 
 	hasEffects(context: HasEffectsContext): boolean {
-		try {
-			for (const argument of this.arguments) {
-				if (argument.hasEffects(context)) return true;
-			}
-			if (this.annotationPure) {
-				return false;
-			}
-			return (
-				this.callee.hasEffects(context) ||
-				this.callee.hasEffectsOnInteractionAtPath(EMPTY_PATH, this.interaction, context)
-			);
-		} finally {
-			if (!this.deoptimized) this.applyDeoptimizations();
+		if (!this.deoptimized) this.applyDeoptimizations();
+		for (const argument of this.arguments) {
+			if (argument.hasEffects(context)) return true;
+		}
+		if (this.annotationPure) {
+			return false;
 		}
+		return (
+			this.callee.hasEffects(context) ||
+			this.callee.hasEffectsOnInteractionAtPath(EMPTY_PATH, this.interaction, context)
+		);
 	}
 
 	hasEffectsOnInteractionAtPath(path: ObjectPath, { type }: NodeInteraction): boolean {

diff --git a/src/ast/nodes/TaggedTemplateExpression.ts b/src/ast/nodes/TaggedTemplateExpression.ts
@@ -34,17 +34,14 @@ export default class TaggedTemplateExpression extends CallExpressionBase {
 	}
 
 	hasEffects(context: HasEffectsContext): boolean {
-		try {
-			for (const argument of this.quasi.expressions) {
-				if (argument.hasEffects(context)) return true;
-			}
-			return (
-				this.tag.hasEffects(context) ||
-				this.tag.hasEffectsOnInteractionAtPath(EMPTY_PATH, this.interaction, context)
-			);
-		} finally {
-			if (!this.deoptimized) this.applyDeoptimizations();
+		if (!this.deoptimized) this.applyDeoptimizations();
+		for (const argument of this.quasi.expressions) {
+			if (argument.hasEffects(context)) return true;
 		}
+		return (
+			this.tag.hasEffects(context) ||
+			this.tag.hasEffectsOnInteractionAtPath(EMPTY_PATH, this.interaction, context)
+		);
 	}
 
 	include(context: InclusionContext, includeChildrenRecursively: IncludeChildren): void {

diff --git a/test/function/samples/recursive-parameter-assignments/main.js b/test/function/samples/recursive-parameter-assignments/main.js
@@ -1,3 +1,5 @@
+let created = false;
+
 class Test {
 	constructor ( name, opts ) {
 		opts = opts || {};
@@ -10,7 +12,10 @@ class Test {
 
 		this.name = name;
 		this.opts = opts;
+		// to make the function call not pure
+		created = true;
 	}
 }
 
 new Test( 'a', {} );
+assert.equal( created, true );
diff --git a/test/function/samples/recursive-parameter-binding/_config.js b/test/function/samples/recursive-parameter-binding/_config.js
@@ -0,0 +1,4 @@
+module.exports = defineTest({
+	description:
+		'Avoid maximum call stack error, make sure parameter value is optimized before hasEffects (#5499).'
+});
diff --git a/test/function/samples/recursive-parameter-binding/main.js b/test/function/samples/recursive-parameter-binding/main.js
@@ -0,0 +1,26 @@
+let created = 0;
+
+function Test(options) {
+	if (!this) {
+		return new Test(options);
+	}
+	if (options.x) {
+		return 0;
+	}
+	created++;
+}
+
+function Test2(options) {
+	if (!this) {
+		return new Test2(options);
+	}
+	if (options.x) {
+		return 0;
+	}
+	created++;
+}
+
+new Test({});
+Test2`{}`;
+
+assert.equal(created, 2);