From dc22a79725bf4a81d3624eabc13cf3cd348832e3 Mon Sep 17 00:00:00 2001 From: Michal Moskal Date: Thu, 21 May 2015 11:42:14 -0700 Subject: [PATCH] disallow extract with return/break/continue; disallow extract with more than one output variable --- ast/ast.ts | 119 +++++++++++++++++++++++++++++++-------------- editor/selector.ts | 19 +++++++- 2 files changed, 100 insertions(+), 38 deletions(-) diff --git a/ast/ast.ts b/ast/ast.ts index 065476ed..414a9092 100644 --- a/ast/ast.ts +++ b/ast/ast.ts @@ -639,21 +639,24 @@ module TDev.AST { public allowAddOf(n:Stmt) { return n instanceof InlineAction } } - export class For + export class Loop extends Stmt + { + public body:CodeBlock; + public primaryBody() { return this.body; } + public isExecutableStmt() { return true; } + public loopVariable():LocalDef { return null } + } + + export class For + extends Loop { public boundLocal:LocalDef; public upperBound:ExprHolder; - public body:CodeBlock; - constructor() { - super() - } public nodeType() { return "for"; } public calcNode() { return this.upperBound; } public children() { return [ this.upperBound, this.body]; } - public primaryBody() { return this.body; } public accept(v:NodeVisitor) { return v.visitFor(this); } - public isExecutableStmt() { return true; } public allowSimplify() { return true } public loopVariable():LocalDef { return this.boundLocal } @@ -681,22 +684,16 @@ module TDev.AST { } export class Foreach - extends Stmt + extends Loop { public boundLocal:LocalDef; public collection:ExprHolder; // the block contains Where stmts only (at the moment; in future there might be OrderBy etc) public conditions:ConditionBlock; - public body:CodeBlock; - constructor() { - super() - } public nodeType() { return "foreach"; } public calcNode() { return this.collection; } - public primaryBody() { return this.body; } public accept(v:NodeVisitor) { return v.visitForeach(this); } public children() { return [ this.collection, this.conditions, this.body]; } - public isExecutableStmt() { return true; } public allowSimplify() { return true } public loopVariable():LocalDef { return this.boundLocal } @@ -737,19 +734,13 @@ module TDev.AST { } export class While - extends Stmt + extends Loop { public condition:ExprHolder; - public body:CodeBlock; - constructor() { - super() - } public nodeType() { return "while"; } public calcNode() { return this.condition; } - public primaryBody() { return this.body; } public accept(v:NodeVisitor) { return v.visitWhile(this); } public children() { return [ this.condition, this.body]; } - public isExecutableStmt() { return true; } public writeTo(tw:TokenWriter) { @@ -2079,6 +2070,7 @@ module TDev.AST { public isRegular:boolean; public isHiddenOut:boolean; public isOut:boolean; + public _lastWriteLocation:Stmt; public writeWithType(app:App, tw:TokenWriter) { @@ -3956,6 +3948,8 @@ module TDev.AST { private writtenLocals0:LocalDef[]; readLocals:LocalDef[]; writtenLocals:LocalDef[]; + lastStmt:Stmt; + saveWrites = false; // globals read or written, including both global variables and records readGlobals: Decl[]; @@ -3965,10 +3959,13 @@ module TDev.AST { getGlobals: boolean = false; - add(l:Decl, lst:LocalDef[]) + add(s:Stmt, l:Decl, lst:LocalDef[]) { - if (l instanceof LocalDef && lst.indexOf(l) < 0) + if (l instanceof LocalDef && lst.indexOf(l) < 0) { + if (this.saveWrites && s) + (l)._lastWriteLocation = s lst.push(l); + } } private getLocal(e:Token):LocalDef @@ -4015,6 +4012,13 @@ module TDev.AST { return null; } + visitStmt(s:Stmt) + { + this.lastStmt = s + this.visitChildren(s) + return null; + } + visitAction(n: Action) { if (this.visitedActions && this.visitedActions.indexOf(n) < 0) { this.visitedActions.push(n); @@ -4024,7 +4028,7 @@ module TDev.AST { visitThingRef(n:ThingRef) { - this.add(n.def, this.readLocals0); + this.add(null, n.def, this.readLocals0); return null; } @@ -4035,7 +4039,7 @@ module TDev.AST { if (prop == api.core.AssignmentProp) { n.args[0].flatten(api.core.TupleProp).forEach((e) => { var l = this.getLocal(e); - if (l) this.add(l, this.writtenLocals0); + if (l) this.add(null, l, this.writtenLocals0); else if (this.getGlobals) { var g = this.getGlobal(e); if (g) this.addGlobal(g, this.writtenGlobals); @@ -4080,12 +4084,14 @@ module TDev.AST { visitInlineAction(n:InlineAction) { - n.inParameters.forEach(i => this.add(i, this.writtenLocals)) + n.inParameters.forEach(i => this.add(n, i, this.writtenLocals)) super.visitInlineAction(n) } visitExprHolder(n:ExprHolder) { + var stmt = this.lastStmt + this.readLocals0 = []; this.writtenLocals0 = []; @@ -4096,24 +4102,24 @@ module TDev.AST { n.tokens.forEach((t) => { var l = this.getLocal(t); if (l && this.readLocals0.indexOf(l) < 0 && this.writtenLocals0.indexOf(l) < 0) - this.add(l, this.readLocals0); + this.add(null, l, this.readLocals0); }); - this.readLocals0.forEach((l) => this.add(l, this.readLocals)); - this.writtenLocals0.forEach((l) => this.add(l, this.writtenLocals)); + this.readLocals0.forEach((l) => this.add(null, l, this.readLocals)); + this.writtenLocals0.forEach((l) => this.add(stmt, l, this.writtenLocals)); return null; } visitFor(n:For) { - this.add(n.boundLocal, this.writtenLocals); + this.add(n, n.boundLocal, this.writtenLocals); super.visitFor(n); return null; } visitForeach(n:Foreach) { - this.add(n.boundLocal, this.writtenLocals); + this.add(n, n.boundLocal, this.writtenLocals); super.visitForeach(n); return null; } @@ -4136,6 +4142,7 @@ module TDev.AST { { extractedAction:Action; numAwait = 0; + failed:Stmt[] = []; constructor(public extracted:CodeBlock, public action:Action, public callPlaceholder:ExprStmt, public name:string) { @@ -4151,20 +4158,47 @@ module TDev.AST { run() { + AST.visitStmtsCtx({ inLoop: false, inHandler: false }, this.extracted, (ctx, s) => { + if (s.calcNode() && s.calcNode().parsed) { + var prop = s.calcNode().parsed.calledProp() + if (!ctx.inLoop && (prop == api.core.BreakProp || prop == api.core.ContinueProp)) + this.failed.push(s) + if (!ctx.inHandler && (prop == api.core.ReturnProp)) + this.failed.push(s) + } + ctx = Util.jsonClone(ctx) + if (s instanceof Loop) + ctx.inLoop = true + if (s instanceof InlineAction) { + ctx.inHandler = true + ctx.inLoop = true + } + return ctx + }) + + if (this.failed.length > 0) return + + this.saveWrites = true this.traverse(this.extracted); var hasAwait = this.numAwait > 0; var readSub = this.readLocals; var writtenSub = this.writtenLocals; + this.saveWrites = false this.traverse(this.action.body); - this.action.getInParameters().forEach((p) => this.add(p.local, this.writtenLocals)); + this.action.getInParameters().forEach((p) => this.add(null, p.local, this.writtenLocals)); if (this.action.modelParameter) - this.add(this.action.modelParameter.local, this.writtenLocals) - this.action.getOutParameters().forEach((p) => this.add(p.local, this.readLocals)); + this.add(null, this.action.modelParameter.local, this.writtenLocals) + this.action.getOutParameters().forEach((p) => this.add(null, p.local, this.readLocals)); var outgoing = writtenSub.filter((l) => this.readLocals.indexOf(l) >= 0); var incoming = readSub.filter((l) => this.writtenLocals.indexOf(l) >= 0); + if (outgoing.length > 1) { + this.failed = outgoing.map(l => l._lastWriteLocation) + return + } + var refLocal = (l:LocalDef) => mkThing(l.getName()); var copyNames:any = {} var extractedStmts = this.extracted.stmts.slice(0); @@ -4586,24 +4620,35 @@ module TDev.AST { } } - class StmtVisitor + class StmtVisitor extends NodeVisitor { - constructor(public f:(s:Stmt)=>void) + t:T; + + constructor(public f:(ctx:T, s:Stmt)=>T) { super() } visitStmt(s:Stmt) { - this.f(s) + var prev = this.t + this.t = this.f(prev, s) this.visitChildren(s) + this.t = prev } } export function visitStmts(s:Stmt, f:(s:Stmt)=>void) + { + var v = new StmtVisitor((ctx, s) => { f(s); return ctx; }) + v.dispatch(s) + } + + export function visitStmtsCtx(ctx:T, s:Stmt, f:(ctx:T, s:Stmt)=>T) { var v = new StmtVisitor(f) + v.t = ctx v.dispatch(s) } diff --git a/editor/selector.ts b/editor/selector.ts index 4e6cb1fd..a94d9f25 100644 --- a/editor/selector.ts +++ b/editor/selector.ts @@ -297,6 +297,20 @@ module TDev callPlaceholder, Script.freshName(this.extractedName.value)); extr.run(); + + if (extr.failed.length > 0) { + var ss = this.selectionBlock.stmts + var ch = ss.slice(0, this.selectionBegin).concat(extracted.stmts).concat(ss.slice(this.selectionBegin + 1, ss.length)) + this.selectionBlock.setChildren(ch); + this.unselect() + extr.failed.forEach(s => { + var elt = s && s.renderedAs + if (elt) + Util.coreAnim("shakeTip", 500, elt) + }) + return + } + TheEditor.initIds(extr.extractedAction) Script.addDecl(extr.extractedAction); TheEditor.queueNavRefresh(); @@ -306,7 +320,10 @@ module TDev if (elt) Util.coreAnim("blinkLocation", 1000, elt); } - public copyOutSelection() { return this.selectionBlock ? this.selectionBlock.stmts.slice(this.selectionBegin, this.selectionEnd + 1) : []; } + public copyOutSelection() + { + return this.selectionBlock ? this.selectionBlock.stmts.slice(this.selectionBegin, this.selectionEnd + 1) : []; + } private replaceSelectionWithPlaceholder() {