Skip to content

Commit

Permalink
Use correct range for error in escape
Browse files Browse the repository at this point in the history
  • Loading branch information
som-snytt committed Apr 11, 2024
1 parent 18b61a9 commit 772bb9f
Show file tree
Hide file tree
Showing 21 changed files with 133 additions and 120 deletions.
1 change: 0 additions & 1 deletion src/compiler/scala/tools/nsc/ast/parser/Parsers.scala
Expand Up @@ -1359,7 +1359,6 @@ self =>
def selector(start: Offset, t0: Tree): Tree = {
val t = stripParens(t0)
val point = if (isIdent) in.offset else in.lastOffset //scala/bug#8459
//assert(t.pos.isDefined, t)
if (t != EmptyTree)
Select(t, ident(skipIt = false)) setPos r2p(start, point, in.lastOffset)
else
Expand Down
14 changes: 10 additions & 4 deletions src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala
Expand Up @@ -188,19 +188,25 @@ trait NamesDefaults { self: Analyzer =>

// never used for constructor calls, they always have a stable qualifier
def blockWithQualifier(qual: Tree, selected: Name) = {
val sym = blockTyper.context.owner.newValue(freshTermName(nme.QUAL_PREFIX)(typer.fresh), newFlags = ARTIFACT) setInfo uncheckedBounds(qual.tpe) setPos (qual.pos.makeTransparent)
val sym = blockTyper.context.owner.newValue(freshTermName(nme.QUAL_PREFIX)(typer.fresh), newFlags = ARTIFACT)
.setInfo(uncheckedBounds(qual.tpe))
.setPos(qual.pos.makeTransparent)
blockTyper.context.scope enter sym
val vd = atPos(sym.pos)(ValDef(sym, qual) setType NoType)
// it stays in Vegas: scala/bug#5720, scala/bug#5727
qual.changeOwner(blockTyper.context.owner, sym)
qual.changeOwner(blockTyper.context.owner, sym) // scala/bug#5720, scala/bug#5727

val newQual = atPos(qual.pos.focus)(blockTyper.typedQualifier(Ident(sym.name)))
val baseFunTransformed = atPos(baseFun.pos.makeTransparent) {
// setSymbol below is important because the 'selected' function might be overloaded. by
// assigning the correct method symbol, typedSelect will just assign the type. the reason
// to still call 'typed' is to correctly infer singleton types, scala/bug#5259.
val selectPos =
if(qual.pos.isRange && baseFun1.pos.isRange) qual.pos.union(baseFun1.pos).withStart(Math.min(qual.pos.end, baseFun1.pos.end))
if (qual.pos.isRange && baseFun1.pos.isRange)
if (qual.pos == baseFun1.pos) qual.pos
else {
val start = Math.min(qual.pos.end, baseFun1.pos.end) // split into ranges that do not overlap
qual.pos.union(baseFun1.pos).withStart(start)
}
else baseFun1.pos
val f = blockTyper.typedOperator(Select(newQual, selected).setSymbol(baseFun1.symbol).setPos(selectPos))
if (funTargs.isEmpty) f
Expand Down
85 changes: 43 additions & 42 deletions src/compiler/scala/tools/reflect/FastStringInterpolator.scala
Expand Up @@ -13,6 +13,8 @@
package scala.tools
package reflect

import scala.StringContext.{InvalidEscapeException, InvalidUnicodeEscapeException, processEscapes, processUnicode}
import scala.reflect.internal.util.Position
import nsc.Reporting.WarningCategory.Scala3Migration

trait FastStringInterpolator extends FormatInterpolator {
Expand All @@ -26,45 +28,46 @@ trait FastStringInterpolator extends FormatInterpolator {
// rewrite a tree like `scala.StringContext.apply("hello \\n ", " ", "").s("world", Test.this.foo)`
// to `"hello \n world ".+(Test.this.foo)`
private def interpolated(macroApp: Tree, isRaw: Boolean): Tree = macroApp match {
case Apply(Select(Apply(stringCtx@Select(qualSC, _), parts), _interpol), args) if
stringCtx.symbol == currentRun.runDefinitions.StringContext_apply &&
treeInfo.isQualifierSafeToElide(qualSC) &&
parts.forall(treeInfo.isLiteralString) &&
parts.length == (args.length + 1) =>
case Apply(Select(Apply(stringCtx @ Select(qualSC, _), parts), _interpol), args)
if stringCtx.symbol == currentRun.runDefinitions.StringContext_apply
&& treeInfo.isQualifierSafeToElide(qualSC)
&& parts.forall(treeInfo.isLiteralString)
&& parts.length == (args.length + 1) =>

val treated =
try
parts.mapConserve {
case lit @ Literal(Constant(stringVal: String)) =>
def asRaw = if (currentRun.sourceFeatures.unicodeEscapesRaw) stringVal else {
val processed = StringContext.processUnicode(stringVal)
if (processed == stringVal) stringVal else {
val pos = {
val diffindex = processed.zip(stringVal).zipWithIndex.collectFirst {
case ((p, o), i) if p != o => i
}.getOrElse(processed.length - 1)
lit.pos.withShift(diffindex)
}
def msg(fate: String) = s"Unicode escapes in raw interpolations are $fate; use literal characters instead"
if (currentRun.isScala3)
runReporting.warning(pos, msg("ignored in Scala 3 (or with -Xsource-features:unicode-escapes-raw)"), Scala3Migration, c.internal.enclosingOwner)
else
runReporting.deprecationWarning(pos, msg("deprecated"), "2.13.2", "", "")
processed
}
def adjustedPos(p: Position, delta: Int) = {
val start = p.start + delta
Position.range(p.source, start = start, point = start, end = start + 2)
}
val treated = parts.mapConserve {
case lit @ Literal(Constant(stringVal: String)) =>
def asRaw = if (currentRun.sourceFeatures.unicodeEscapesRaw) stringVal else {
val processed = processUnicode(stringVal)
if (processed == stringVal) stringVal else {
val pos = {
val diffindex = processed.zip(stringVal).zipWithIndex.collectFirst {
case ((p, o), i) if p != o => i
}.getOrElse(processed.length - 1)
lit.pos.withShift(diffindex)
}
val value =
if (isRaw) asRaw
else StringContext.processEscapes(stringVal)
val k = Constant(value)
// To avoid the backlash of backslash, taken literally by Literal, escapes are processed strictly (scala/bug#11196)
treeCopy.Literal(lit, k).setType(ConstantType(k))
case x => throw new MatchError(x)
def msg(fate: String) = s"Unicode escapes in raw interpolations are $fate; use literal characters instead"
if (currentRun.isScala3)
runReporting.warning(pos, msg("ignored in Scala 3 (or with -Xsource-features:unicode-escapes-raw)"), Scala3Migration, c.internal.enclosingOwner)
else
runReporting.deprecationWarning(pos, msg("deprecated"), "2.13.2", "", "")
processed
}
}
catch {
case ie: StringContext.InvalidEscapeException => c.abort(parts.head.pos.withShift(ie.index), ie.getMessage)
case iue: StringContext.InvalidUnicodeEscapeException => c.abort(parts.head.pos.withShift(iue.index), iue.getMessage)
}
val value =
try if (isRaw) asRaw else processEscapes(stringVal)
catch {
case ie: InvalidEscapeException => c.abort(adjustedPos(lit.pos, ie.index), ie.getMessage)
case iue: InvalidUnicodeEscapeException => c.abort(adjustedPos(lit.pos, iue.index), iue.getMessage)
}
val k = Constant(value)
// To avoid the backlash of backslash, taken literally by Literal, escapes are processed strictly (scala/bug#11196)
treeCopy.Literal(lit, k).setType(ConstantType(k))
case x => throw new MatchError(x)
}

if (args.forall(treeInfo.isLiteralString)) {
val it1 = treated.iterator
Expand All @@ -82,7 +85,7 @@ trait FastStringInterpolator extends FormatInterpolator {
else concatenate(treated, args)

// Fallback -- inline the original implementation of the `s` or `raw` interpolator.
case t@Apply(Select(someStringContext, _interpol), args) =>
case Apply(Select(someStringContext, _interpol), args) =>
q"""{
val sc = $someStringContext
_root_.scala.StringContext.standardInterpolator(
Expand All @@ -103,20 +106,19 @@ trait FastStringInterpolator extends FormatInterpolator {
if (i < numLits - 1) {
concatArgs += argsIndexed(i)
if (!emptyLit) concatArgs += lit
} else if (!emptyLit) {
concatArgs += lit
}
else if (!emptyLit) concatArgs += lit
}
def mkConcat(pos: Position, lhs: Tree, rhs: Tree): Tree =
atPos(pos)(gen.mkMethodCall(gen.mkAttributedSelect(lhs, definitions.String_+), rhs :: Nil)).setType(definitions.StringTpe)

var result: Tree = parts.head
val chunkSize = 32
if (concatArgs.lengthCompare(chunkSize) <= 0) {
if (concatArgs.lengthCompare(chunkSize) <= 0)
concatArgs.foreach { t =>
result = mkConcat(t.pos, result, t)
}
} else {
else
concatArgs.toList.grouped(chunkSize).foreach {
case group =>
var chunkResult: Tree = Literal(Constant("")).setType(definitions.StringTpe)
Expand All @@ -125,7 +127,6 @@ trait FastStringInterpolator extends FormatInterpolator {
}
result = mkConcat(chunkResult.pos, result, chunkResult)
}
}

result
}
Expand Down
67 changes: 41 additions & 26 deletions src/reflect/scala/reflect/internal/Positions.scala
Expand Up @@ -46,23 +46,34 @@ trait Positions extends api.Positions { self: SymbolTable =>
* Otherwise returns default position that is either focused or not.
*/
def wrappingPos(default: Position, trees: List[Tree]): Position = wrappingPos(default, trees, focus = true)
def wrappingPos(default: Position, trees: List[Tree], focus: Boolean): Position = {
if (useOffsetPositions) default else {
val accum = new WrappingPosAccumulator()
var rest = trees
while (rest ne Nil) {
val head = rest.head
rest = rest.tail
// TODO: a tree's range position should cover the positions of all trees it "includes"
// (inclusion mostly refers to subtrees, but also other attributes reached through the tree, such as its annotations/modifiers);
// concretely, a MemberDef's position should cover its annotations (scala/bug#11060)
// Workaround, which explicitly includes annotations of traversed trees, can be removed when TODO above is resolved:
head match { case md: MemberDef => rest = md.mods.annotations ::: rest case _ => }

accum(head)
}
accum.result(default, focus)

/** If the default point falls outside the calculated range, widen the result to include it.
*/
def wideningPos(default: Position, trees: List[Tree]): Position = {
val res = wrappingPos(default, trees)
val pointed = default.isDefined && default.start <= default.point && default.point <= default.end // has a point
if (pointed && (default.point < res.start || default.point > res.end)) {
val start = Math.min(res.start, default.point)
val end = Math.max(res.end, default.point)
Position.range(default.source, start = start, point = default.point, end = end)
}
else res
}
def wrappingPos(default: Position, trees: List[Tree], focus: Boolean): Position = if (useOffsetPositions) default else {
val accum = new WrappingPosAccumulator()
var rest = trees
while (rest ne Nil) {
val head = rest.head
rest = rest.tail
// TODO: a tree's range position should cover the positions of all trees it "includes"
// (inclusion mostly refers to subtrees, but also other attributes reached through the tree, such as its annotations/modifiers);
// concretely, a MemberDef's position should cover its annotations (scala/bug#11060)
// Workaround, which explicitly includes annotations of traversed trees, can be removed when TODO above is resolved:
head match { case md: MemberDef => rest = md.mods.annotations ::: rest case _ => }

accum(head)
}
accum.result(default, focus)
}
private final class WrappingPosAccumulator extends (Tree => Boolean) {
private[this] var min: Int = _
Expand All @@ -72,11 +83,16 @@ trait Positions extends api.Positions { self: SymbolTable =>
max = Int.MinValue
}
reset()
def result(default: Position, focus: Boolean): Position = {
if (min > max)
if (focus) default.focus else default //there are no ranges
else Position.range(default.source, min, default.pointOrElse(min), max)
}
def result(default: Position, focus: Boolean): Position =
if (min > max) // there are no ranges
if (focus) default.focus else default
else { // ignore default.point not included by range min..max
val pointed = default.isDefined && default.start <= default.point && default.point <= default.end // has a point
val point =
if (pointed && min <= default.point && default.point <= max) default.point
else min
Position.range(default.source, start = min, point = point, end = max)
}
override def apply(v1: Tree): Boolean = {
val pos = v1.pos
if (pos.isRange) {
Expand All @@ -103,9 +119,8 @@ trait Positions extends api.Positions { self: SymbolTable =>
* shortening the range, assigning TransparentPositions
* to some of the nodes in `tree` or focusing on the position.
*/
def ensureNonOverlapping(tree: Tree, others: List[Tree]): Unit ={ ensureNonOverlapping(tree, others, focus = true) }
def ensureNonOverlapping(tree: Tree, others: List[Tree], focus: Boolean): Unit = {
if (useOffsetPositions) return
def ensureNonOverlapping(tree: Tree, others: List[Tree]): Unit = ensureNonOverlapping(tree, others, focus = true)
def ensureNonOverlapping(tree: Tree, others: List[Tree], focus: Boolean): Unit = if (!useOffsetPositions) {

def isOverlapping(pos: Position) =
pos.isRange && (others exists (pos overlaps _.pos))
Expand Down Expand Up @@ -305,15 +320,15 @@ trait Positions extends api.Positions { self: SymbolTable =>
def set(pos: Position, parent: Tree): Unit = {
wrappingPosAccumulator.reset()
this.pos = pos
try parent.foreachChild(this)
try parent.foreachChild(apply)
finally {
this.pos = null
}
}
def apply(tree: Tree): Boolean = {
wrappingPosAccumulator.reset()
if (!tree.isEmpty && tree.canHaveAttrs && tree.pos == NoPosition) {
tree.foreachChild(this)
tree.foreachChild(apply)
tree.foreachChild(wrappingPosAccumulator)
val wrappingPos = wrappingPosAccumulator.result(pos, focus = true)
tree setPos wrappingPos
Expand Down
7 changes: 2 additions & 5 deletions src/reflect/scala/reflect/internal/TreeGen.scala
Expand Up @@ -423,13 +423,10 @@ abstract class TreeGen {
if (vparamss1.isEmpty || !vparamss1.head.isEmpty && vparamss1.head.head.mods.isImplicit)
vparamss1 = List() :: vparamss1
val superCall = pendingSuperCall // we can't know in advance which of the parents will end up as a superclass
// this requires knowing which of the parents is a type macro and which is not
// and that's something that cannot be found out before typer
// (the type macros aren't in the trunk yet, but there is a plan for them to land there soon)
// this means that we don't know what will be the arguments of the super call
// therefore here we emit a dummy which gets populated when the template is named and typechecked
// here we emit a dummy which gets populated when the template is named and typechecked
Some(
atPos(wrappingPos(superPos, lvdefs ::: vparamss1.flatten).makeTransparent) (
atPos(wideningPos(superPos, lvdefs ::: vparamss1.flatten).makeTransparent) (
DefDef(constrMods, nme.CONSTRUCTOR, List(), vparamss1, TypeTree(), Block(lvdefs ::: List(superCall), mkLiteralUnit))))
}
}
Expand Down
8 changes: 5 additions & 3 deletions src/reflect/scala/reflect/internal/util/Position.scala
Expand Up @@ -37,9 +37,11 @@ object Position {
final val tabInc = 8

private def validate[T <: Position](pos: T): T = {
if (pos.isRange)
assert(pos.start <= pos.end, s"bad position: ${pos.show}")

if (pos.isRange) {
import pos.{pos => _, _}
assert(start <= end, s"bad position: ${pos.show}")
//assert(start <= point && point < end, s"bad position: point $point out of range $start..$end: ${pos.show}")
}
pos
}

Expand Down
4 changes: 2 additions & 2 deletions test/files/neg/annots-constant-neg.check
Expand Up @@ -3,7 +3,7 @@ Test.scala:12: error: class Ann1 cannot have auxiliary constructors because it e
^
Test.scala:14: error: class Ann2 needs to have exactly one argument list because it extends ConstantAnnotation
class Ann2(x: Int)(y: Int) extends ConstantAnnotation // err
^
^
Test.scala:27: error: annotation argument needs to be a constant; found: Test.this.nonConst
@JAnn(nonConst) def t3 = 0 // err
^
Expand Down Expand Up @@ -68,7 +68,7 @@ Test.scala:66: error: annotation argument needs to be a constant; found: java.la
Test.scala:69: error: Java annotation SuppressWarnings is abstract; cannot be instantiated
Error occurred in an application involving default arguments.
@Ann(value = 0, c = Array(new SuppressWarnings(value = Array("")))) def u17 = 0 // err
^
^
Test.scala:69: error: not found: value value
Error occurred in an application involving default arguments.
@Ann(value = 0, c = Array(new SuppressWarnings(value = Array("")))) def u17 = 0 // err
Expand Down
4 changes: 2 additions & 2 deletions test/files/neg/java-annotation-bad.check
Expand Up @@ -3,13 +3,13 @@ Test_1.scala:12: error: Java annotation Ann_0 is abstract; cannot be instantiate
^
Test_1.scala:13: error: Java annotation Ann_0 is abstract; cannot be instantiated
val b: Ann_0 = new Ann_0(Array()) // nok
^
^
Test_1.scala:14: error: Java annotation Ann_1 is abstract; cannot be instantiated
val c: Ann_1 = new Ann_1 // nok
^
Test_1.scala:15: error: Java annotation Ann_1 is abstract; cannot be instantiated
val d: Ann_1 = new Ann_1(Array()) // nok
^
^
Test_1.scala:18: error: type mismatch;
found : ann.Ann_0
required: scala.annotation.Annotation
Expand Down
2 changes: 1 addition & 1 deletion test/files/neg/protected-constructors.check
Expand Up @@ -15,7 +15,7 @@ protected-constructors.scala:18: error: constructor Foo2 in class Foo2 cannot be
enclosing object P in package hungus is not a subclass of
class Foo2 in package dingus where target is defined
val foo2 = new Foo2("abc")
^
^
protected-constructors.scala:19: error: class Foo3 in object Ding cannot be accessed as a member of object dingus.Ding from object P in package hungus
Access to protected class Foo3 not permitted because
enclosing object P in package hungus is not a subclass of
Expand Down
2 changes: 1 addition & 1 deletion test/files/neg/t10514.check
Expand Up @@ -4,7 +4,7 @@ argument expression's type is not compatible with formal parameter type;
found : Some[C[Foo[Option,Test.this.Id]]]
required: ?F[C[?G[Foo[?F,?G]]]]
new Foo(Some(new C(new Foo[Option, Id](None))))
^
^
t10514.scala:8: error: type mismatch;
found : Some[C[Foo[Option,Test.this.Id]]]
required: F[C[G[Foo[F,G]]]]
Expand Down
2 changes: 1 addition & 1 deletion test/files/neg/t12155.check
@@ -1,4 +1,4 @@
t12155.scala:6: error: class Node in class C1 cannot be accessed as a member of C1[A,_$1] from package <empty>
class D[A, C](val x: C1[A, _]#Node[C]) {
^
^
1 error
2 changes: 1 addition & 1 deletion test/files/neg/t4987.check
@@ -1,4 +1,4 @@
t4987.scala:2: error: constructor Foo2 in class Foo2 cannot be accessed in object Bar2 from object Bar2
object Bar2 { new Foo2(0, 0) }
^
^
1 error
12 changes: 6 additions & 6 deletions test/files/neg/t5696.check
@@ -1,19 +1,19 @@
t5696.scala:6: error: too many argument lists for constructor invocation
new G(1)(2) {}
^
^
t5696.scala:14: error: too many argument lists for constructor invocation
new G()(2) {}
^
^
t5696.scala:22: error: too many argument lists for constructor invocation
new G[Int]()(2) {}
^
^
t5696.scala:30: error: too many argument lists for constructor invocation
new G[Int]()(2)(3) {}
^
^
t5696.scala:38: error: too many argument lists for constructor invocation
new G[Int]()()(2) {}
^
^
t5696.scala:46: error: too many argument lists for constructor invocation
object x extends G(1)(2) {}
^
^
6 errors

0 comments on commit 772bb9f

Please sign in to comment.