From 19873df87e1de47e80fde2d86740611c0732abb3 Mon Sep 17 00:00:00 2001 From: TrickyPi <530257315@qq.com> Date: Fri, 14 Apr 2023 00:29:18 +0800 Subject: [PATCH 1/4] fix: add a new InclusionContext prop existedBroken --- src/ast/ExecutionContext.ts | 2 ++ src/ast/nodes/IfStatement.ts | 4 ++++ src/ast/nodes/SwitchCase.ts | 10 +++++++- .../_expected.js | 23 +++++++++++++++++++ .../break-statement-labels-switch/main.js | 23 +++++++++++++++++++ 5 files changed, 61 insertions(+), 1 deletion(-) diff --git a/src/ast/ExecutionContext.ts b/src/ast/ExecutionContext.ts index dfb3cefd493..39d0ae662a5 100644 --- a/src/ast/ExecutionContext.ts +++ b/src/ast/ExecutionContext.ts @@ -21,6 +21,7 @@ interface ControlFlowContext { } export interface InclusionContext extends ControlFlowContext { + existedBroken: boolean; includedCallArguments: Set; } @@ -37,6 +38,7 @@ export interface HasEffectsContext extends ControlFlowContext { export function createInclusionContext(): InclusionContext { return { brokenFlow: BROKEN_FLOW_NONE, + existedBroken: false, includedCallArguments: new Set(), includedLabels: new Set() }; diff --git a/src/ast/nodes/IfStatement.ts b/src/ast/nodes/IfStatement.ts index 2049d253aba..23f41b21f97 100644 --- a/src/ast/nodes/IfStatement.ts +++ b/src/ast/nodes/IfStatement.ts @@ -2,6 +2,7 @@ import type MagicString from 'magic-string'; import type { RenderOptions } from '../../utils/renderHelpers'; import type { DeoptimizableEntity } from '../DeoptimizableEntity'; import { + BROKEN_FLOW_BREAK_CONTINUE, BROKEN_FLOW_NONE, type HasEffectsContext, type InclusionContext @@ -173,10 +174,13 @@ export default class IfStatement extends StatementBase implements DeoptimizableE this.consequent.include(context, false, { asSingleStatement: true }); // eslint-disable-next-line unicorn/consistent-destructuring consequentBrokenFlow = context.brokenFlow; + context.existedBroken ||= consequentBrokenFlow === BROKEN_FLOW_BREAK_CONTINUE; context.brokenFlow = brokenFlow; } if (this.alternate?.shouldBeIncluded(context)) { this.alternate.include(context, false, { asSingleStatement: true }); + // eslint-disable-next-line unicorn/consistent-destructuring + context.existedBroken ||= context.brokenFlow === BROKEN_FLOW_BREAK_CONTINUE; context.brokenFlow = // eslint-disable-next-line unicorn/consistent-destructuring context.brokenFlow < consequentBrokenFlow ? context.brokenFlow : consequentBrokenFlow; diff --git a/src/ast/nodes/SwitchCase.ts b/src/ast/nodes/SwitchCase.ts index ab8392964ab..47556a63ed3 100644 --- a/src/ast/nodes/SwitchCase.ts +++ b/src/ast/nodes/SwitchCase.ts @@ -5,7 +5,12 @@ import { type RenderOptions, renderStatementList } from '../../utils/renderHelpers'; -import type { HasEffectsContext, InclusionContext } from '../ExecutionContext'; +import { + BROKEN_FLOW_BREAK_CONTINUE, + BROKEN_FLOW_ERROR_RETURN_LABEL, + type HasEffectsContext, + type InclusionContext +} from '../ExecutionContext'; import type * as NodeType from './NodeType'; import { type ExpressionNode, @@ -36,6 +41,9 @@ export default class SwitchCase extends NodeBase { if (includeChildrenRecursively || node.shouldBeIncluded(context)) node.include(context, includeChildrenRecursively); } + if (context.existedBroken && context.brokenFlow === BROKEN_FLOW_ERROR_RETURN_LABEL) { + context.brokenFlow = BROKEN_FLOW_BREAK_CONTINUE; + } } render(code: MagicString, options: RenderOptions, nodeRenderOptions?: NodeRenderOptions): void { diff --git a/test/form/samples/break-control-flow/break-statement-labels-switch/_expected.js b/test/form/samples/break-control-flow/break-statement-labels-switch/_expected.js index c020736c988..0a0f0b7da5f 100644 --- a/test/form/samples/break-control-flow/break-statement-labels-switch/_expected.js +++ b/test/form/samples/break-control-flow/break-statement-labels-switch/_expected.js @@ -74,3 +74,26 @@ function empty() { } empty(); + +function issue(obj) { + switch (obj.field1) { + case 'baz': + switch (obj.field2) { + case 'value': { + if (obj.field3) { + if (obj.field4) { + break; + } + } + throw new Error(`error 1`); + } + default: + throw new Error(`error 2`); + } + break; + default: + throw new Error('error 3'); + } +} + +issue({ field1: 'baz', field2: 'value' }); diff --git a/test/form/samples/break-control-flow/break-statement-labels-switch/main.js b/test/form/samples/break-control-flow/break-statement-labels-switch/main.js index 480a547205f..94e39043620 100644 --- a/test/form/samples/break-control-flow/break-statement-labels-switch/main.js +++ b/test/form/samples/break-control-flow/break-statement-labels-switch/main.js @@ -116,3 +116,26 @@ function empty() { } empty(); + +function issue(obj) { + switch (obj.field1) { + case 'baz': + switch (obj.field2) { + case 'value': { + if (obj.field3) { + if (obj.field4) { + break; + } + } + throw new Error(`error 1`); + } + default: + throw new Error(`error 2`); + } + break; + default: + throw new Error('error 3'); + } +} + +issue({ field1: 'baz', field2: 'value' }); From 07261712c9207b3b681d13908ca1e423b14477d0 Mon Sep 17 00:00:00 2001 From: TrickyPi <530257315@qq.com> Date: Fri, 14 Apr 2023 21:51:19 +0800 Subject: [PATCH 2/4] test: tweak test --- .../_expected.js | 23 ------------------- .../break-statement-labels-switch/main.js | 23 ------------------- test/function/samples/switch-break/_config.js | 3 +++ test/function/samples/switch-break/main.js | 21 +++++++++++++++++ 4 files changed, 24 insertions(+), 46 deletions(-) create mode 100644 test/function/samples/switch-break/_config.js create mode 100644 test/function/samples/switch-break/main.js diff --git a/test/form/samples/break-control-flow/break-statement-labels-switch/_expected.js b/test/form/samples/break-control-flow/break-statement-labels-switch/_expected.js index 0a0f0b7da5f..c020736c988 100644 --- a/test/form/samples/break-control-flow/break-statement-labels-switch/_expected.js +++ b/test/form/samples/break-control-flow/break-statement-labels-switch/_expected.js @@ -74,26 +74,3 @@ function empty() { } empty(); - -function issue(obj) { - switch (obj.field1) { - case 'baz': - switch (obj.field2) { - case 'value': { - if (obj.field3) { - if (obj.field4) { - break; - } - } - throw new Error(`error 1`); - } - default: - throw new Error(`error 2`); - } - break; - default: - throw new Error('error 3'); - } -} - -issue({ field1: 'baz', field2: 'value' }); diff --git a/test/form/samples/break-control-flow/break-statement-labels-switch/main.js b/test/form/samples/break-control-flow/break-statement-labels-switch/main.js index 94e39043620..480a547205f 100644 --- a/test/form/samples/break-control-flow/break-statement-labels-switch/main.js +++ b/test/form/samples/break-control-flow/break-statement-labels-switch/main.js @@ -116,26 +116,3 @@ function empty() { } empty(); - -function issue(obj) { - switch (obj.field1) { - case 'baz': - switch (obj.field2) { - case 'value': { - if (obj.field3) { - if (obj.field4) { - break; - } - } - throw new Error(`error 1`); - } - default: - throw new Error(`error 2`); - } - break; - default: - throw new Error('error 3'); - } -} - -issue({ field1: 'baz', field2: 'value' }); diff --git a/test/function/samples/switch-break/_config.js b/test/function/samples/switch-break/_config.js new file mode 100644 index 00000000000..9e60c85325e --- /dev/null +++ b/test/function/samples/switch-break/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'if switch does not always throw an error, retain the following break' +}; diff --git a/test/function/samples/switch-break/main.js b/test/function/samples/switch-break/main.js new file mode 100644 index 00000000000..7643a17a0ff --- /dev/null +++ b/test/function/samples/switch-break/main.js @@ -0,0 +1,21 @@ +function issue(obj) { + switch (obj.field1) { + case 'baz': + switch (obj.field2) { + case 'value': { + if (obj.field1) { + if (obj.field1) { + break; + } + } + throw new Error(`error 1`); + } + default: + throw new Error(`error 2`); + } + break; // retained + default: + throw new Error('error 3'); + } +} +issue({ field1: 'baz', field2: 'value' }); From bb923fcec947e9106bce0e20d70621854d1fd109 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Sun, 16 Apr 2023 13:32:56 +0200 Subject: [PATCH 3/4] Rework using labels --- src/ast/ExecutionContext.ts | 26 +++++------- src/ast/nodes/ArrowFunctionExpression.ts | 2 - src/ast/nodes/BreakStatement.ts | 23 ++++------ src/ast/nodes/ContinueStatement.ts | 21 ++++----- src/ast/nodes/DoWhileStatement.ts | 15 ++----- src/ast/nodes/ForInStatement.ts | 15 ++----- src/ast/nodes/ForOfStatement.ts | 5 +-- src/ast/nodes/ForStatement.ts | 18 +++----- src/ast/nodes/IfStatement.ts | 20 +++------ src/ast/nodes/ReturnStatement.ts | 10 ++--- src/ast/nodes/SwitchCase.ts | 10 +---- src/ast/nodes/SwitchStatement.ts | 52 ++++++++++++++--------- src/ast/nodes/ThrowStatement.ts | 4 +- src/ast/nodes/TryStatement.ts | 4 +- src/ast/nodes/WhileStatement.ts | 15 ++----- src/ast/nodes/shared/FunctionBase.ts | 8 +--- src/ast/nodes/shared/FunctionNode.ts | 2 - src/ast/nodes/shared/loops.ts | 54 ++++++++++++++++++++++++ 18 files changed, 143 insertions(+), 161 deletions(-) create mode 100644 src/ast/nodes/shared/loops.ts diff --git a/src/ast/ExecutionContext.ts b/src/ast/ExecutionContext.ts index 39d0ae662a5..26f3a20cfe1 100644 --- a/src/ast/ExecutionContext.ts +++ b/src/ast/ExecutionContext.ts @@ -3,32 +3,29 @@ import type { ExpressionEntity } from './nodes/shared/Expression'; import { DiscriminatedPathTracker, PathTracker } from './utils/PathTracker'; import type ThisVariable from './variables/ThisVariable'; +export const UnlabeledBreak = Symbol('Unlabeled Break'); +export const UnlabeledContinue = Symbol('Unlabeled Continue'); +export type Label = string | typeof UnlabeledBreak | typeof UnlabeledContinue; + interface ExecutionContextIgnore { - breaks: boolean; - continues: boolean; - labels: Set; + labels: Set