From 40c1913dd539beb4b6184b37d87e48d98803a465 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 17 Nov 2016 18:29:09 +0100 Subject: [PATCH 1/3] More robust scheme for taking start/end of positions when parsing Some trees, which do not consume input have unassigned positions (so that they can fit in whatever range they are integrated). It's therefore risky to take the start or end of a parsed tree's position. This commit guards against the case where the position of the tree does not exist. --- .../dotty/tools/dotc/parsing/Parsers.scala | 71 +++++++++++-------- tests/neg/i1705.scala | 24 +++++++ 2 files changed, 64 insertions(+), 31 deletions(-) create mode 100644 tests/neg/i1705.scala diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index fa0576c7a212..838f7514c423 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -83,6 +83,14 @@ object Parsers { def atPos[T <: Positioned](start: Offset)(t: T): T = atPos(start, start)(t) + /** Defensive version of Position#start */ + def startPos(t: Positioned): Int = + if (t.pos.exists) t.pos.start else in.offset + + /** Defensive version of Position#end */ + def endPos(t: Positioned): Int = + if (t.pos.exists) t.pos.end else in.lastOffset + def nameStart: Offset = if (in.token == BACKQUOTED_IDENT) in.offset + 1 else in.offset @@ -448,7 +456,7 @@ object Parsers { val topInfo = opStack.head opStack = opStack.tail val od = reduceStack(base, topInfo.operand, 0, true) - return atPos(od.pos.start, topInfo.offset) { + return atPos(startPos(od), topInfo.offset) { PostfixOp(od, topInfo.operator) } } @@ -492,7 +500,7 @@ object Parsers { /** Accept identifier acting as a selector on given tree `t`. */ def selector(t: Tree): Tree = - atPos(t.pos.start, in.offset) { Select(t, ident()) } + atPos(startPos(t), in.offset) { Select(t, ident()) } /** Selectors ::= ident { `.' ident() * @@ -728,7 +736,7 @@ object Parsers { def refinedTypeRest(t: Tree): Tree = { newLineOptWhenFollowedBy(LBRACE) - if (in.token == LBRACE) refinedTypeRest(atPos(t.pos.start) { RefinedTypeTree(t, refinement()) }) + if (in.token == LBRACE) refinedTypeRest(atPos(startPos(t)) { RefinedTypeTree(t, refinement()) }) else t } @@ -749,7 +757,7 @@ object Parsers { def annotType(): Tree = annotTypeRest(simpleType()) def annotTypeRest(t: Tree): Tree = - if (in.token == AT) annotTypeRest(atPos(t.pos.start) { Annotated(t, annot()) }) + if (in.token == AT) annotTypeRest(atPos(startPos(t)) { Annotated(t, annot()) }) else t /** SimpleType ::= SimpleType TypeArgs @@ -780,19 +788,19 @@ object Parsers { val handleSingletonType: Tree => Tree = t => if (in.token == TYPE) { in.nextToken() - atPos(t.pos.start) { SingletonTypeTree(t) } + atPos(startPos(t)) { SingletonTypeTree(t) } } else t private def simpleTypeRest(t: Tree): Tree = in.token match { case HASH => simpleTypeRest(typeProjection(t)) - case LBRACKET => simpleTypeRest(atPos(t.pos.start) { AppliedTypeTree(t, typeArgs(namedOK = true)) }) + case LBRACKET => simpleTypeRest(atPos(startPos(t)) { AppliedTypeTree(t, typeArgs(namedOK = true)) }) case _ => t } private def typeProjection(t: Tree): Tree = { accept(HASH) val id = typeIdent() - atPos(t.pos.start, id.pos.start) { Select(t, id.name) } + atPos(startPos(t), startPos(id)) { Select(t, id.name) } } /** NamedTypeArg ::= id `=' Type @@ -846,7 +854,7 @@ object Parsers { val t = toplevelTyp() if (isIdent(nme.raw.STAR)) { in.nextToken() - atPos(t.pos.start) { PostfixOp(t, nme.raw.STAR) } + atPos(startPos(t)) { PostfixOp(t, nme.raw.STAR) } } else t } @@ -971,7 +979,7 @@ object Parsers { val t = expr1(location) if (in.token == ARROW) { placeholderParams = saved - closureRest(t.pos.start, location, convertToParams(t)) + closureRest(startPos(t), location, convertToParams(t)) } else if (isWildcard(t)) { placeholderParams = placeholderParams ::: saved @@ -1025,7 +1033,7 @@ object Parsers { assert(handlerStart != -1) syntaxError( new EmptyCatchBlock(body), - Position(handlerStart, handler.pos.end) + Position(handlerStart, endPos(handler)) ) case _ => } @@ -1035,7 +1043,7 @@ object Parsers { else { if (handler.isEmpty) warning( EmptyCatchAndFinallyBlock(body), - source atPos Position(tryOffset, body.pos.end) + source atPos Position(tryOffset, endPos(body)) ) EmptyTree } @@ -1057,21 +1065,21 @@ object Parsers { case EQUALS => t match { case Ident(_) | Select(_, _) | Apply(_, _) => - atPos(t.pos.start, in.skipToken()) { Assign(t, expr()) } + atPos(startPos(t), in.skipToken()) { Assign(t, expr()) } case _ => t } case COLON => ascription(t, location) case MATCH => - atPos(t.pos.start, in.skipToken()) { + atPos(startPos(t), in.skipToken()) { inBraces(Match(t, caseClauses())) } case _ => t } - def ascription(t: Tree, location: Location.Value) = atPos(t.pos.start, in.skipToken()) { + def ascription(t: Tree, location: Location.Value) = atPos(startPos(t), in.skipToken()) { in.token match { case USCORE => val uscoreStart = in.skipToken() @@ -1105,7 +1113,7 @@ object Parsers { val id = termIdent() val paramExpr = if (location == Location.InBlock && in.token == COLON) - atPos(id.pos.start, in.skipToken()) { Typed(id, infixType()) } + atPos(startPos(id), in.skipToken()) { Typed(id, infixType()) } else id closureRest(start, location, convertToParam(paramExpr, mods) :: Nil) @@ -1194,13 +1202,13 @@ object Parsers { in.nextToken() simpleExprRest(selector(t), canApply = true) case LBRACKET => - val tapp = atPos(t.pos.start, in.offset) { TypeApply(t, typeArgs(namedOK = true)) } + val tapp = atPos(startPos(t), in.offset) { TypeApply(t, typeArgs(namedOK = true)) } simpleExprRest(tapp, canApply = true) case LPAREN | LBRACE if canApply => - val app = atPos(t.pos.start, in.offset) { Apply(t, argumentExprs()) } + val app = atPos(startPos(t), in.offset) { Apply(t, argumentExprs()) } simpleExprRest(app, canApply = true) case USCORE => - atPos(t.pos.start, in.skipToken()) { PostfixOp(t, nme.WILDCARD) } + atPos(startPos(t), in.skipToken()) { PostfixOp(t, nme.WILDCARD) } case _ => t } @@ -1284,7 +1292,7 @@ object Parsers { if (in.token == IF) guard() else { val pat = pattern1() - if (in.token == EQUALS) atPos(pat.pos.start, in.skipToken()) { GenAlias(pat, expr()) } + if (in.token == EQUALS) atPos(startPos(pat), in.skipToken()) { GenAlias(pat, expr()) } else generatorRest(pat) } @@ -1293,7 +1301,7 @@ object Parsers { def generator(): Tree = generatorRest(pattern1()) def generatorRest(pat: Tree) = - atPos(pat.pos.start, accept(LARROW)) { GenFrom(pat, expr()) } + atPos(startPos(pat), accept(LARROW)) { GenFrom(pat, expr()) } /** ForExpr ::= `for' (`(' Enumerators `)' | `{' Enumerators `}') * {nl} [`yield'] Expr @@ -1357,7 +1365,7 @@ object Parsers { val pattern = () => { val pat = pattern1() if (isIdent(nme.raw.BAR)) - atPos(pat.pos.start) { Alternative(pat :: patternAlts()) } + atPos(startPos(pat)) { Alternative(pat :: patternAlts()) } else pat } @@ -1383,15 +1391,15 @@ object Parsers { // compatibility for Scala2 `x @ _*` syntax infixPattern() match { case pt @ Ident(tpnme.WILDCARD_STAR) => - migrationWarningOrError("The syntax `x @ _*' is no longer supported; use `x : _*' instead", p.pos.start) - atPos(p.pos.start, offset) { Typed(p, pt) } + migrationWarningOrError("The syntax `x @ _*' is no longer supported; use `x : _*' instead", startPos(p)) + atPos(startPos(p), offset) { Typed(p, pt) } case p => - atPos(p.pos.start, offset) { Bind(name, p) } + atPos(startPos(p), offset) { Bind(name, p) } } case p @ Ident(tpnme.WILDCARD_STAR) => // compatibility for Scala2 `_*` syntax - migrationWarningOrError("The syntax `_*' is no longer supported; use `x : _*' instead", p.pos.start) - atPos(p.pos.start) { Typed(Ident(nme.WILDCARD), p) } + migrationWarningOrError("The syntax `_*' is no longer supported; use `x : _*' instead", startPos(p)) + atPos(startPos(p)) { Typed(Ident(nme.WILDCARD), p) } case p => p } @@ -1415,7 +1423,7 @@ object Parsers { val simplePattern = () => in.token match { case IDENTIFIER | BACKQUOTED_IDENT | THIS => path(thisOK = true) match { - case id @ Ident(nme.raw.MINUS) if isNumericLit => literal(id.pos.start) + case id @ Ident(nme.raw.MINUS) if isNumericLit => literal(startPos(id)) case t => simplePatternRest(t) } case USCORE => @@ -1445,9 +1453,9 @@ object Parsers { def simplePatternRest(t: Tree): Tree = { var p = t if (in.token == LBRACKET) - p = atPos(t.pos.start, in.offset) { TypeApply(p, typeArgs()) } + p = atPos(startPos(t), in.offset) { TypeApply(p, typeArgs()) } if (in.token == LPAREN) - p = atPos(t.pos.start, in.offset) { Apply(p, argumentPatterns()) } + p = atPos(startPos(t), in.offset) { Apply(p, argumentPatterns()) } p } @@ -1573,7 +1581,8 @@ object Parsers { case Select(qual, name) => cpy.Select(tree)(adjustStart(start)(qual), name) case _ => tree } - if (start < tree1.pos.start) tree1.withPos(tree1.pos.withStart(start)) + if (tree1.pos.exists && start < tree1.pos.start) + tree1.withPos(tree1.pos.withStart(start)) else tree1 } @@ -1804,7 +1813,7 @@ object Parsers { def importSelector(): Tree = { val from = termIdentOrWildcard() if (from.name != nme.WILDCARD && in.token == ARROW) - atPos(from.pos.start, in.skipToken()) { + atPos(startPos(from), in.skipToken()) { Thicket(from, termIdentOrWildcard()) } else from diff --git a/tests/neg/i1705.scala b/tests/neg/i1705.scala new file mode 100644 index 000000000000..801adb4b766f --- /dev/null +++ b/tests/neg/i1705.scala @@ -0,0 +1,24 @@ +object DepBug { + class A { + class B + def mkB = new B + def m(b: B) = b + } + trait Dep { + val a: A + val b: a.B + } + val dep = new { + val a = new A + val b = a mkB + } + def useDep(d: Dep) { // error: procedure syntax + import d._ + a m (b) + } + { // error: Null does not take parameters (follow on) + import dep._ + a m (b) + } + dep.a m (dep b) // error (follow on) +} From 18eea6b3159e8caaffc1e415ce69301b64d566d4 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 17 Nov 2016 18:35:08 +0100 Subject: [PATCH 2/3] Rename ...Pos to ...Offset and add pointOffset --- .../dotty/tools/dotc/parsing/Parsers.scala | 73 ++++++++++--------- 1 file changed, 37 insertions(+), 36 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 838f7514c423..51dafc9289eb 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -83,12 +83,13 @@ object Parsers { def atPos[T <: Positioned](start: Offset)(t: T): T = atPos(start, start)(t) - /** Defensive version of Position#start */ - def startPos(t: Positioned): Int = + def startOffset(t: Positioned): Int = if (t.pos.exists) t.pos.start else in.offset - /** Defensive version of Position#end */ - def endPos(t: Positioned): Int = + def pointOffset(t: Positioned): Int = + if (t.pos.exists) t.pos.point else in.offset + + def endOffset(t: Positioned): Int = if (t.pos.exists) t.pos.end else in.lastOffset def nameStart: Offset = @@ -456,7 +457,7 @@ object Parsers { val topInfo = opStack.head opStack = opStack.tail val od = reduceStack(base, topInfo.operand, 0, true) - return atPos(startPos(od), topInfo.offset) { + return atPos(startOffset(od), topInfo.offset) { PostfixOp(od, topInfo.operator) } } @@ -500,7 +501,7 @@ object Parsers { /** Accept identifier acting as a selector on given tree `t`. */ def selector(t: Tree): Tree = - atPos(startPos(t), in.offset) { Select(t, ident()) } + atPos(startOffset(t), in.offset) { Select(t, ident()) } /** Selectors ::= ident { `.' ident() * @@ -736,7 +737,7 @@ object Parsers { def refinedTypeRest(t: Tree): Tree = { newLineOptWhenFollowedBy(LBRACE) - if (in.token == LBRACE) refinedTypeRest(atPos(startPos(t)) { RefinedTypeTree(t, refinement()) }) + if (in.token == LBRACE) refinedTypeRest(atPos(startOffset(t)) { RefinedTypeTree(t, refinement()) }) else t } @@ -757,7 +758,7 @@ object Parsers { def annotType(): Tree = annotTypeRest(simpleType()) def annotTypeRest(t: Tree): Tree = - if (in.token == AT) annotTypeRest(atPos(startPos(t)) { Annotated(t, annot()) }) + if (in.token == AT) annotTypeRest(atPos(startOffset(t)) { Annotated(t, annot()) }) else t /** SimpleType ::= SimpleType TypeArgs @@ -788,19 +789,19 @@ object Parsers { val handleSingletonType: Tree => Tree = t => if (in.token == TYPE) { in.nextToken() - atPos(startPos(t)) { SingletonTypeTree(t) } + atPos(startOffset(t)) { SingletonTypeTree(t) } } else t private def simpleTypeRest(t: Tree): Tree = in.token match { case HASH => simpleTypeRest(typeProjection(t)) - case LBRACKET => simpleTypeRest(atPos(startPos(t)) { AppliedTypeTree(t, typeArgs(namedOK = true)) }) + case LBRACKET => simpleTypeRest(atPos(startOffset(t)) { AppliedTypeTree(t, typeArgs(namedOK = true)) }) case _ => t } private def typeProjection(t: Tree): Tree = { accept(HASH) val id = typeIdent() - atPos(startPos(t), startPos(id)) { Select(t, id.name) } + atPos(startOffset(t), startOffset(id)) { Select(t, id.name) } } /** NamedTypeArg ::= id `=' Type @@ -854,7 +855,7 @@ object Parsers { val t = toplevelTyp() if (isIdent(nme.raw.STAR)) { in.nextToken() - atPos(startPos(t)) { PostfixOp(t, nme.raw.STAR) } + atPos(startOffset(t)) { PostfixOp(t, nme.raw.STAR) } } else t } @@ -979,7 +980,7 @@ object Parsers { val t = expr1(location) if (in.token == ARROW) { placeholderParams = saved - closureRest(startPos(t), location, convertToParams(t)) + closureRest(startOffset(t), location, convertToParams(t)) } else if (isWildcard(t)) { placeholderParams = placeholderParams ::: saved @@ -1033,7 +1034,7 @@ object Parsers { assert(handlerStart != -1) syntaxError( new EmptyCatchBlock(body), - Position(handlerStart, endPos(handler)) + Position(handlerStart, endOffset(handler)) ) case _ => } @@ -1043,7 +1044,7 @@ object Parsers { else { if (handler.isEmpty) warning( EmptyCatchAndFinallyBlock(body), - source atPos Position(tryOffset, endPos(body)) + source atPos Position(tryOffset, endOffset(body)) ) EmptyTree } @@ -1065,21 +1066,21 @@ object Parsers { case EQUALS => t match { case Ident(_) | Select(_, _) | Apply(_, _) => - atPos(startPos(t), in.skipToken()) { Assign(t, expr()) } + atPos(startOffset(t), in.skipToken()) { Assign(t, expr()) } case _ => t } case COLON => ascription(t, location) case MATCH => - atPos(startPos(t), in.skipToken()) { + atPos(startOffset(t), in.skipToken()) { inBraces(Match(t, caseClauses())) } case _ => t } - def ascription(t: Tree, location: Location.Value) = atPos(startPos(t), in.skipToken()) { + def ascription(t: Tree, location: Location.Value) = atPos(startOffset(t), in.skipToken()) { in.token match { case USCORE => val uscoreStart = in.skipToken() @@ -1113,7 +1114,7 @@ object Parsers { val id = termIdent() val paramExpr = if (location == Location.InBlock && in.token == COLON) - atPos(startPos(id), in.skipToken()) { Typed(id, infixType()) } + atPos(startOffset(id), in.skipToken()) { Typed(id, infixType()) } else id closureRest(start, location, convertToParam(paramExpr, mods) :: Nil) @@ -1202,13 +1203,13 @@ object Parsers { in.nextToken() simpleExprRest(selector(t), canApply = true) case LBRACKET => - val tapp = atPos(startPos(t), in.offset) { TypeApply(t, typeArgs(namedOK = true)) } + val tapp = atPos(startOffset(t), in.offset) { TypeApply(t, typeArgs(namedOK = true)) } simpleExprRest(tapp, canApply = true) case LPAREN | LBRACE if canApply => - val app = atPos(startPos(t), in.offset) { Apply(t, argumentExprs()) } + val app = atPos(startOffset(t), in.offset) { Apply(t, argumentExprs()) } simpleExprRest(app, canApply = true) case USCORE => - atPos(startPos(t), in.skipToken()) { PostfixOp(t, nme.WILDCARD) } + atPos(startOffset(t), in.skipToken()) { PostfixOp(t, nme.WILDCARD) } case _ => t } @@ -1292,7 +1293,7 @@ object Parsers { if (in.token == IF) guard() else { val pat = pattern1() - if (in.token == EQUALS) atPos(startPos(pat), in.skipToken()) { GenAlias(pat, expr()) } + if (in.token == EQUALS) atPos(startOffset(pat), in.skipToken()) { GenAlias(pat, expr()) } else generatorRest(pat) } @@ -1301,7 +1302,7 @@ object Parsers { def generator(): Tree = generatorRest(pattern1()) def generatorRest(pat: Tree) = - atPos(startPos(pat), accept(LARROW)) { GenFrom(pat, expr()) } + atPos(startOffset(pat), accept(LARROW)) { GenFrom(pat, expr()) } /** ForExpr ::= `for' (`(' Enumerators `)' | `{' Enumerators `}') * {nl} [`yield'] Expr @@ -1365,7 +1366,7 @@ object Parsers { val pattern = () => { val pat = pattern1() if (isIdent(nme.raw.BAR)) - atPos(startPos(pat)) { Alternative(pat :: patternAlts()) } + atPos(startOffset(pat)) { Alternative(pat :: patternAlts()) } else pat } @@ -1391,15 +1392,15 @@ object Parsers { // compatibility for Scala2 `x @ _*` syntax infixPattern() match { case pt @ Ident(tpnme.WILDCARD_STAR) => - migrationWarningOrError("The syntax `x @ _*' is no longer supported; use `x : _*' instead", startPos(p)) - atPos(startPos(p), offset) { Typed(p, pt) } + migrationWarningOrError("The syntax `x @ _*' is no longer supported; use `x : _*' instead", startOffset(p)) + atPos(startOffset(p), offset) { Typed(p, pt) } case p => - atPos(startPos(p), offset) { Bind(name, p) } + atPos(startOffset(p), offset) { Bind(name, p) } } case p @ Ident(tpnme.WILDCARD_STAR) => // compatibility for Scala2 `_*` syntax - migrationWarningOrError("The syntax `_*' is no longer supported; use `x : _*' instead", startPos(p)) - atPos(startPos(p)) { Typed(Ident(nme.WILDCARD), p) } + migrationWarningOrError("The syntax `_*' is no longer supported; use `x : _*' instead", startOffset(p)) + atPos(startOffset(p)) { Typed(Ident(nme.WILDCARD), p) } case p => p } @@ -1423,7 +1424,7 @@ object Parsers { val simplePattern = () => in.token match { case IDENTIFIER | BACKQUOTED_IDENT | THIS => path(thisOK = true) match { - case id @ Ident(nme.raw.MINUS) if isNumericLit => literal(startPos(id)) + case id @ Ident(nme.raw.MINUS) if isNumericLit => literal(startOffset(id)) case t => simplePatternRest(t) } case USCORE => @@ -1453,9 +1454,9 @@ object Parsers { def simplePatternRest(t: Tree): Tree = { var p = t if (in.token == LBRACKET) - p = atPos(startPos(t), in.offset) { TypeApply(p, typeArgs()) } + p = atPos(startOffset(t), in.offset) { TypeApply(p, typeArgs()) } if (in.token == LPAREN) - p = atPos(startPos(t), in.offset) { Apply(p, argumentPatterns()) } + p = atPos(startOffset(t), in.offset) { Apply(p, argumentPatterns()) } p } @@ -1780,7 +1781,7 @@ object Parsers { case imp: Import => imp case sel @ Select(qual, name) => - val selector = atPos(sel.pos.point) { Ident(name) } + val selector = atPos(pointOffset(sel)) { Ident(name) } cpy.Import(sel)(qual, selector :: Nil) case t => accept(DOT) @@ -1813,7 +1814,7 @@ object Parsers { def importSelector(): Tree = { val from = termIdentOrWildcard() if (from.name != nme.WILDCARD && in.token == ARROW) - atPos(startPos(from), in.skipToken()) { + atPos(startOffset(from), in.skipToken()) { Thicket(from, termIdentOrWildcard()) } else from @@ -2094,7 +2095,7 @@ object Parsers { /** Create a tree representing a packaging */ def makePackaging(start: Int, pkg: Tree, stats: List[Tree]): PackageDef = pkg match { - case x: RefTree => atPos(start, pkg.pos.point)(PackageDef(x, stats)) + case x: RefTree => atPos(start, pointOffset(pkg))(PackageDef(x, stats)) } /** Packaging ::= package QualId [nl] `{' TopStatSeq `}' From 3d20226ab4c5e4425edc30901a9746d4bb46e105 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 24 Nov 2016 16:45:26 +0100 Subject: [PATCH 3/3] Renamed test file to give correct issue number --- tests/neg/{i1705.scala => i1707.scala} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/neg/{i1705.scala => i1707.scala} (100%) diff --git a/tests/neg/i1705.scala b/tests/neg/i1707.scala similarity index 100% rename from tests/neg/i1705.scala rename to tests/neg/i1707.scala