diff --git a/lib/coffeescript/nodes.js b/lib/coffeescript/nodes.js index 3a920e0060..1c79988675 100644 --- a/lib/coffeescript/nodes.js +++ b/lib/coffeescript/nodes.js @@ -617,12 +617,18 @@ Base.prototype.includeCommentFragments = NO; // `jumps` tells you if an expression, or an internal part of an expression - // has a flow control construct (like `break`, or `continue`, or `return`, - // or `throw`) that jumps out of the normal flow of control and can’t be - // used as a value. This is important because things like this make no sense; - // we have to disallow them. + // has a flow control construct (like `break`, `continue`, or `return`) + // that jumps out of the normal flow of control and can’t be used as a value. + // This is important because things like this make no sense; + // we have to disallow them. Note that `throw` *is* allowed here though. Base.prototype.jumps = NO; + // `alwaysJumps` tells you whether this node *always* has a flow control + // construct (like `break`, `continue`, `return`, or `throw`) that jumps out + // of the normal flow of control, so that the immediately following node + // definitely won't execute. + Base.prototype.alwaysJumps = NO; + // If `node.shouldCache() is false`, it is safe to use `node` more than once. // Otherwise you need to store the value of `node` in a variable and output // that variable several times instead. Kind of like this: `5` need not be @@ -869,6 +875,20 @@ } } + // Block is executed in sequence, so if any statement always jumps, + // then so does the block. + alwaysJumps(o) { + var exp, j, len1, ref1; + ref1 = this.expressions; + for (j = 0, len1 = ref1.length; j < len1; j++) { + exp = ref1[j]; + if (exp.alwaysJumps(o)) { + return true; + } + } + return false; + } + // A Block node does not return its entire body, rather it // ensures that the final expression is returned. makeReturn(results, mark) { @@ -1714,6 +1734,10 @@ } } + alwaysJumps(o) { + return Boolean(this.jumps(o)); + } + compileNode(o) { return [this.makeCode(`${this.tab}${this.value};`)]; } @@ -1901,6 +1925,8 @@ Return.prototype.jumps = THIS; + Return.prototype.alwaysJumps = YES; + return Return; }).call(this); @@ -2075,6 +2101,10 @@ return !this.properties.length && this.base.jumps(o); } + alwaysJumps(o) { + return !this.properties.length && this.base.alwaysJumps(o); + } + isObject(onlyGenerated) { if (this.properties.length) { return false; @@ -4810,6 +4840,8 @@ ModuleDeclaration.prototype.jumps = THIS; + ModuleDeclaration.prototype.alwaysJumps = NO; + ModuleDeclaration.prototype.makeReturn = THIS; return ModuleDeclaration; @@ -6425,6 +6457,8 @@ Code.prototype.jumps = NO; + Code.prototype.alwaysJumps = NO; + return Code; }).call(this); @@ -6813,6 +6847,23 @@ return false; } + alwaysJumps() { + var expressions, j, len1, node; + ({expressions} = this.body); + if (!expressions.length) { + return false; + } + for (j = 0, len1 = expressions.length; j < len1; j++) { + node = expressions[j]; + if (node.alwaysJumps({ + loop: true + })) { + return true; + } + } + return false; + } + // The main difference from a JavaScript *while* is that the CoffeeScript // *while* can be used as a part of a larger expression -- while loops may // return an array containing the computed result of each iteration. @@ -7394,6 +7445,11 @@ return this.attempt.jumps(o) || ((ref1 = this.catch) != null ? ref1.jumps(o) : void 0); } + alwaysJumps(o) { + var ref1, ref2; + return (this.attempt.alwaysJumps(o) && ((ref1 = this.catch) != null ? ref1.alwaysJumps(o) : void 0)) || ((ref2 = this.ensure) != null ? ref2.alwaysJumps(o) : void 0); + } + makeReturn(results, mark) { var ref1, ref2; if (mark) { @@ -7472,6 +7528,10 @@ return this.recovery.jumps(o); } + alwaysJumps(o) { + return this.recovery.alwaysJumps(o); + } + makeReturn(results, mark) { var ret; ret = this.recovery.makeReturn(results, mark); @@ -7578,6 +7638,8 @@ Throw.prototype.jumps = NO; + Throw.prototype.alwaysJumps = YES; + // A **Throw** is already a return, of sorts... Throw.prototype.makeReturn = THIS; @@ -8290,6 +8352,23 @@ return (ref2 = this.otherwise) != null ? ref2.jumps(o) : void 0; } + alwaysJumps(o = { + block: true + }) { + var block, j, len1, ref1; + if (!this.cases.length) { + return false; + } + ref1 = this.cases; + for (j = 0, len1 = ref1.length; j < len1; j++) { + ({block} = ref1[j]); + if (!block.alwaysJumps(o)) { + return false; + } + } + return true; + } + makeReturn(results, mark) { var block, j, len1, ref1, ref2; ref1 = this.cases; @@ -8307,7 +8386,7 @@ } compileNode(o) { - var block, body, cond, conditions, expr, fragments, i, idt1, idt2, j, k, len1, len2, ref1, ref2; + var block, body, cond, conditions, fragments, i, idt1, idt2, j, k, len1, len2, ref1, ref2; idt1 = o.indent + TAB; idt2 = o.indent = idt1 + TAB; fragments = [].concat(this.makeCode(this.tab + "switch ("), (this.subject ? this.subject.compileToFragments(o, LEVEL_PAREN) : this.makeCode("false")), this.makeCode(") {\n")); @@ -8328,8 +8407,8 @@ if (i === this.cases.length - 1 && !this.otherwise) { break; } - expr = this.lastNode(block.expressions); - if (expr instanceof Return || expr instanceof Throw || (expr instanceof Literal && expr.jumps() && expr.value !== 'debugger')) { + if (block.alwaysJumps()) { + //expr = @lastNode block.expressions continue; } fragments.push(cond.makeCode(idt2 + 'break;\n')); @@ -8511,6 +8590,11 @@ return this.body.jumps(o) || ((ref1 = this.elseBody) != null ? ref1.jumps(o) : void 0); } + alwaysJumps(o) { + var ref1; + return this.body.alwaysJumps(o) && ((ref1 = this.elseBody) != null ? ref1.alwaysJumps(o) : void 0); + } + compileNode(o) { if (this.isStatement(o)) { return this.compileStatement(o); diff --git a/lib/coffeescript/rewriter.js b/lib/coffeescript/rewriter.js index a2816f0b05..d99b3314bd 100644 --- a/lib/coffeescript/rewriter.js +++ b/lib/coffeescript/rewriter.js @@ -496,7 +496,6 @@ } else { return startIndex; } - break; case this.tag(i - 2) !== '@': return i - 2; default: diff --git a/src/nodes.coffee b/src/nodes.coffee index 63ecaad3d5..d6836db5ec 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -393,12 +393,18 @@ exports.Base = class Base includeCommentFragments: NO # `jumps` tells you if an expression, or an internal part of an expression - # has a flow control construct (like `break`, or `continue`, or `return`, - # or `throw`) that jumps out of the normal flow of control and can’t be - # used as a value. This is important because things like this make no sense; - # we have to disallow them. + # has a flow control construct (like `break`, `continue`, or `return`) + # that jumps out of the normal flow of control and can’t be used as a value. + # This is important because things like this make no sense; + # we have to disallow them. Note that `throw` *is* allowed here though. jumps: NO + # `alwaysJumps` tells you whether this node *always* has a flow control + # construct (like `break`, `continue`, `return`, or `throw`) that jumps out + # of the normal flow of control, so that the immediately following node + # definitely won't execute. + alwaysJumps: NO + # If `node.shouldCache() is false`, it is safe to use `node` more than once. # Otherwise you need to store the value of `node` in a variable and output # that variable several times instead. Kind of like this: `5` need not be @@ -600,6 +606,13 @@ exports.Block = class Block extends Base for exp in @expressions return jumpNode if jumpNode = exp.jumps o + # Block is executed in sequence, so if any statement always jumps, + # then so does the block. + alwaysJumps: (o) -> + for exp in @expressions + return yes if exp.alwaysJumps o + no + # A Block node does not return its entire body, rather it # ensures that the final expression is returned. makeReturn: (results, mark) -> @@ -1198,6 +1211,8 @@ exports.StatementLiteral = class StatementLiteral extends Literal return this if @value is 'break' and not (o?.loop or o?.block) return this if @value is 'continue' and not o?.loop + alwaysJumps: (o) -> Boolean @jumps o + compileNode: (o) -> [@makeCode "#{@tab}#{@value};"] @@ -1269,6 +1284,7 @@ exports.Return = class Return extends Base isStatement: YES makeReturn: THIS jumps: THIS + alwaysJumps: YES compileToFragments: (o, level) -> expr = @expression?.makeReturn() @@ -1398,6 +1414,7 @@ exports.Value = class Value extends Base isJSXTag : -> @base instanceof JSXTag assigns : (name) -> not @properties.length and @base.assigns name jumps : (o) -> not @properties.length and @base.jumps o + alwaysJumps : (o) -> not @properties.length and @base.alwaysJumps o isObject: (onlyGenerated) -> return no if @properties.length @@ -3211,6 +3228,7 @@ exports.ModuleDeclaration = class ModuleDeclaration extends Base isStatement: YES jumps: THIS + alwaysJumps: NO makeReturn: THIS checkSource: -> @@ -3896,6 +3914,7 @@ exports.Code = class Code extends Base isStatement: -> @isMethod jumps: NO + alwaysJumps: NO makeScope: (parentScope) -> new Scope parentScope, @body, this @@ -4555,6 +4574,13 @@ exports.While = class While extends Base return jumpNode if jumpNode = node.jumps loop: yes no + alwaysJumps: -> + {expressions} = @body + return no unless expressions.length + for node in expressions + return yes if node.alwaysJumps loop: yes + no + # The main difference from a JavaScript *while* is that the CoffeeScript # *while* can be used as a part of a larger expression -- while loops may # return an array containing the computed result of each iteration. @@ -4931,6 +4957,10 @@ exports.Try = class Try extends Base jumps: (o) -> @attempt.jumps(o) or @catch?.jumps(o) + alwaysJumps: (o) -> + (@attempt.alwaysJumps(o) and @catch?.alwaysJumps(o)) or + @ensure?.alwaysJumps(o) + makeReturn: (results, mark) -> if mark @attempt?.makeReturn results, mark @@ -4988,7 +5018,9 @@ exports.Catch = class Catch extends Base isStatement: YES - jumps: (o) -> @recovery.jumps(o) + jumps: (o) -> @recovery.jumps o + + alwaysJumps: (o) -> @recovery.alwaysJumps o makeReturn: (results, mark) -> ret = @recovery.makeReturn results, mark @@ -5037,6 +5069,7 @@ exports.Throw = class Throw extends Base isStatement: YES jumps: NO + alwaysJumps: YES # A **Throw** is already a return, of sorts... makeReturn: THIS @@ -5498,6 +5531,12 @@ exports.Switch = class Switch extends Base return jumpNode if jumpNode = block.jumps o @otherwise?.jumps o + alwaysJumps: (o = {block: yes}) -> + return no unless @cases.length + for {block} in @cases + return no unless block.alwaysJumps o + yes + makeReturn: (results, mark) -> block.makeReturn(results, mark) for {block} in @cases @otherwise or= new Block [new Literal 'void 0'] if results @@ -5516,8 +5555,7 @@ exports.Switch = class Switch extends Base fragments = fragments.concat @makeCode(idt1 + "case "), cond.compileToFragments(o, LEVEL_PAREN), @makeCode(":\n") fragments = fragments.concat body, @makeCode('\n') if (body = block.compileToFragments o, LEVEL_TOP).length > 0 break if i is @cases.length - 1 and not @otherwise - expr = @lastNode block.expressions - continue if expr instanceof Return or expr instanceof Throw or (expr instanceof Literal and expr.jumps() and expr.value isnt 'debugger') + continue if block.alwaysJumps() fragments.push cond.makeCode(idt2 + 'break;\n') if @otherwise and @otherwise.expressions.length fragments.push @makeCode(idt1 + "default:\n"), (@otherwise.compileToFragments o, LEVEL_TOP)..., @makeCode("\n") @@ -5615,6 +5653,8 @@ exports.If = class If extends Base jumps: (o) -> @body.jumps(o) or @elseBody?.jumps(o) + alwaysJumps: (o) -> @body.alwaysJumps(o) and @elseBody?.alwaysJumps(o) + compileNode: (o) -> if @isStatement o then @compileStatement o else @compileExpression o diff --git a/test/control_flow.coffee b/test/control_flow.coffee index 7d994aa2a4..94268edac0 100644 --- a/test/control_flow.coffee +++ b/test/control_flow.coffee @@ -440,6 +440,22 @@ test "Issue #997. Switch doesn't fallthrough.", -> eq val, 1 +test "Issue #5344. Switch doesn't generate unnecessary break statements.", -> + js = CoffeeScript.compile ''' + identify = (n) -> + switch n + when 1, 2, 3, 4, 5 + if n % 2 == 0 + 'small even' + else + 'small odd' + when 0 + 'perfect' + else + null + ''' + ok not js.includes 'break' + # Throw test "Throw should be usable as an expression.", ->