Skip to content

Optimize and simplify SourcePosition handling #9561

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions compiler/src/dotty/tools/dotc/ast/Desugar.scala
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ object desugar {
appliedRef(enumClassRef)
else {
report.error(TypedCaseDoesNotExplicitlyExtendTypedEnum(enumClass, cdef)
, cdef.sourcePos.startPos)
, cdef.srcPos.startPos)
appliedTypeTree(enumClassRef, constrTparams map (_ => anyRef))
}

Expand Down Expand Up @@ -633,7 +633,7 @@ object desugar {
.withSpan(cdef.span).toList
if (companionDerived.nonEmpty)
for (modClsDef @ TypeDef(_, _) <- mdefs)
modClsDef.putAttachment(DerivingCompanion, impl.sourcePos.startPos)
modClsDef.putAttachment(DerivingCompanion, impl.srcPos.startPos)
mdefs
}

Expand Down Expand Up @@ -741,19 +741,19 @@ object desugar {
if (!mods.isOneOf(GivenOrImplicit))
Nil
else if (ctx.owner.is(Package)) {
report.error(TopLevelImplicitClass(cdef), cdef.sourcePos)
report.error(TopLevelImplicitClass(cdef), cdef.srcPos)
Nil
}
else if (mods.is(Trait)) {
report.error(TypesAndTraitsCantBeImplicit(), cdef.sourcePos)
report.error(TypesAndTraitsCantBeImplicit(), cdef.srcPos)
Nil
}
else if (isCaseClass) {
report.error(ImplicitCaseClass(cdef), cdef.sourcePos)
report.error(ImplicitCaseClass(cdef), cdef.srcPos)
Nil
}
else if (arity != 1 && !mods.is(Given)) {
report.error(ImplicitClassPrimaryConstructorArity(), cdef.sourcePos)
report.error(ImplicitClassPrimaryConstructorArity(), cdef.srcPos)
Nil
}
else {
Expand Down Expand Up @@ -895,7 +895,7 @@ object desugar {
.withSpan(mdef.span.startPos)
val ValDef(selfName, selfTpt, _) = impl.self
val selfMods = impl.self.mods
if (!selfTpt.isEmpty) report.error(ObjectMayNotHaveSelfType(mdef), impl.self.sourcePos)
if (!selfTpt.isEmpty) report.error(ObjectMayNotHaveSelfType(mdef), impl.self.srcPos)
val clsSelf = ValDef(selfName, SingletonTypeTree(Ident(moduleName)), impl.self.rhs)
.withMods(selfMods)
.withSpan(impl.self.span.orElse(impl.span.startPos))
Expand All @@ -911,7 +911,7 @@ object desugar {
for mdef <- ext.methods yield
if mdef.tparams.nonEmpty then
report.error("extension method cannot have type parameters here, all type parameters go after `extension`",
mdef.tparams.head.sourcePos)
mdef.tparams.head.srcPos)
defDef(
cpy.DefDef(mdef)(
name = mdef.name.toExtensionName,
Expand Down Expand Up @@ -1015,7 +1015,7 @@ object desugar {
case Some(DefDef(name, _, (vparam :: _) :: _, _, _)) =>
s"extension_${name}_${inventTypeName(vparam.tpt)}"
case _ =>
report.error(AnonymousInstanceCannotBeEmpty(impl), impl.sourcePos)
report.error(AnonymousInstanceCannotBeEmpty(impl), impl.srcPos)
nme.ERROR.toString
else
impl.parents.map(inventTypeName(_)).mkString("given_", "_", "")
Expand Down Expand Up @@ -1189,7 +1189,7 @@ object desugar {
var tested: MemberDef = tree
def checkApplicable(flag: Flag, test: MemberDefTest): Unit =
if (tested.mods.is(flag) && !test.applyOrElse(tree, (md: MemberDef) => false)) {
report.error(ModifierNotAllowedForDefinition(flag), tree.sourcePos)
report.error(ModifierNotAllowedForDefinition(flag), tree.srcPos)
tested = tested.withMods(tested.mods.withoutFlags(flag))
}
checkApplicable(Opaque, legalOpaque)
Expand Down Expand Up @@ -1701,7 +1701,7 @@ object desugar {
|This can be achieved by adding the import clause 'import scala.language.postfixOps'
|or by setting the compiler option -language:postfixOps.
|See the Scaladoc for value scala.language.postfixOps for a discussion
|why the feature needs to be explicitly enabled.""".stripMargin, t.sourcePos)
|why the feature needs to be explicitly enabled.""".stripMargin, t.srcPos)
}
Select(t, op.name)
}
Expand Down Expand Up @@ -1821,7 +1821,7 @@ object desugar {
elems foreach collect
case Alternative(trees) =>
for (tree <- trees; (vble, _) <- getVariables(tree))
report.error(IllegalVariableInPatternAlternative(), vble.sourcePos)
report.error(IllegalVariableInPatternAlternative(), vble.srcPos)
case Annotated(arg, _) =>
collect(arg)
case InterpolatedString(_, segments) =>
Expand All @@ -1844,7 +1844,7 @@ object desugar {
def traverse(tree: untpd.Tree)(using Context): Unit = tree match {
case Splice(expr) => collect(expr)
case TypSplice(expr) =>
report.error(TypeSpliceInValPattern(expr), tree.sourcePos)
report.error(TypeSpliceInValPattern(expr), tree.srcPos)
case _ => traverseChildren(tree)
}
}.traverse(expr)
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/ast/DesugarEnums.scala
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ object DesugarEnums {
case Ident(name) =>
val matches = tparamNames.contains(name)
if (matches && (caseTypeParams.nonEmpty || vparamss.isEmpty))
report.error(i"illegal reference to type parameter $name from enum case", tree.sourcePos)
report.error(i"illegal reference to type parameter $name from enum case", tree.srcPos)
matches
case LambdaTypeTree(lambdaParams, body) =>
underBinders(lambdaParams, foldOver(x, tree))
Expand Down
11 changes: 8 additions & 3 deletions compiler/src/dotty/tools/dotc/ast/Positioned.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package dotc
package ast

import util.Spans._
import util.{SourceFile, NoSource, SourcePosition}
import util.{SourceFile, NoSource, SourcePosition, SrcPos}
import core.Contexts._
import core.Decorators._
import core.Flags.{JavaDefined, Extension}
Expand All @@ -17,7 +17,7 @@ import java.io.{ PrintWriter }

/** A base class for things that have positions (currently: modifiers and trees)
*/
abstract class Positioned(implicit @constructorOnly src: SourceFile) extends Product with Cloneable {
abstract class Positioned(implicit @constructorOnly src: SourceFile) extends SrcPos, Product, Cloneable {

private var myUniqueId: Int = _
private var mySpan: Span = _
Expand Down Expand Up @@ -46,7 +46,12 @@ abstract class Positioned(implicit @constructorOnly src: SourceFile) extends Pro
span = envelope(src)

def source: SourceFile = SourceFile.fromId(uniqueId)
def sourcePos: SourcePosition = source.atSpan(span)
def sourcePos(using Context): SourcePosition = source.atSpan(span)

/** This positioned item, widened to `SrcPos`. Used to make clear we only need the
* position, typically for error reporting.
*/
final def srcPos: SrcPos = this

/** A positioned item like this one with given `span`.
* If the positioned item is source-derived, a clone is returned.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ class TreeMapWithImplicits extends tpd.TreeMap {
}
catch {
case ex: TypeError =>
report.error(ex, tree.sourcePos)
report.error(ex, tree.srcPos)
tree
}
}
Expand Down
12 changes: 2 additions & 10 deletions compiler/src/dotty/tools/dotc/ast/Trees.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import language.higherKinds
import collection.mutable.ListBuffer
import printing.Printer
import printing.Texts.Text
import util.{Stats, Attachment, Property, SourceFile, NoSource, SourcePosition}
import util.{Stats, Attachment, Property, SourceFile, NoSource, SrcPos, SourcePosition}
import config.Config
import annotation.internal.sharable
import annotation.unchecked.uncheckedVariance
Expand Down Expand Up @@ -54,18 +54,10 @@ object Trees {
* nodes.
*/
abstract class Tree[-T >: Untyped](implicit @constructorOnly src: SourceFile)
extends Positioned
with Product
with Attachment.Container
with printing.Showable {
extends Positioned, SrcPos, Product, Attachment.Container, printing.Showable {

if (Stats.enabled) ntrees += 1

/** This tree, widened to `Positioned`. Used to make clear we only need the
* position, typically for error reporting.
*/
final def posd: Positioned = this

/** The type constructor at the root of the tree */
type ThisTree[T >: Untyped] <: Tree[T]

Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/config/Feature.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import core._
import Contexts._, Symbols._, Names._, NameOps._, Phases._
import StdNames.nme
import Decorators.{given _}
import util.SourcePosition
import util.SrcPos
import SourceVersion._
import reporting.Message

Expand Down Expand Up @@ -76,7 +76,7 @@ object Feature:
/** If current source migrates to `version`, issue given warning message
* and return `true`, otherwise return `false`.
*/
def warnOnMigration(msg: Message, pos: SourcePosition,
def warnOnMigration(msg: Message, pos: SrcPos,
version: SourceVersion = defaultSourceVersion)(using Context): Boolean =
if sourceVersion.isMigrating && sourceVersion.stable == version
|| version == `3.0` && migrateTo3
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/Decorators.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ package core
import annotation.tailrec
import Symbols._
import Contexts._, Names._, Phases._, printing.Texts._, printing.Printer
import util.Spans.Span, util.SourcePosition
import util.Spans.Span
import collection.mutable.ListBuffer
import dotty.tools.dotc.transform.MegaPhase
import ast.tpd._
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2508,7 +2508,7 @@ object SymDenotations {
def complete(denot: SymDenotation)(using Context): Unit = {
val sym = denot.symbol
val errMsg = BadSymbolicReference(denot)
report.error(errMsg, sym.sourcePos)
report.error(errMsg, sym.srcPos)
if (ctx.debug) throw new scala.Error()
initializeToDefaults(denot, errMsg)
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/SymbolLoaders.scala
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ object SymbolLoaders {
report.warning(i"""$what ${tree.name} is in the wrong directory.
|It was declared to be in package ${path.reverse.mkString(".")}
|But it is found in directory ${filePath.reverse.mkString(File.separator)}""",
tree.sourcePos.focus)
tree.srcPos.focus)
ok
}

Expand Down
9 changes: 7 additions & 2 deletions compiler/src/dotty/tools/dotc/core/Symbols.scala
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import reporting.Message
import collection.mutable
import io.AbstractFile
import language.implicitConversions
import util.{SourceFile, NoSource, Property, SourcePosition}
import util.{SourceFile, NoSource, Property, SourcePosition, SrcPos}
import scala.collection.JavaConverters._
import scala.annotation.internal.sharable
import config.Printers.typr
Expand All @@ -47,7 +47,7 @@ object Symbols {
* @param id A unique identifier of the symbol (unique per ContextBase)
*/
class Symbol private[Symbols] (private var myCoord: Coord, val id: Int)
extends Designator with ParamInfo with printing.Showable {
extends Designator, ParamInfo, SrcPos, printing.Showable {

type ThisName <: Name

Expand Down Expand Up @@ -321,6 +321,11 @@ object Symbols {
(if (src.exists) src else ctx.source).atSpan(span)
}

/** This positioned item, widened to `SrcPos`. Used to make clear we only need the
* position, typically for error reporting.
*/
final def srcPos: SrcPos = this

// ParamInfo types and methods
def isTypeParam(using Context): Boolean = denot.is(TypeParam)
def paramName(using Context): ThisName = name.asInstanceOf[ThisName]
Expand Down
1 change: 0 additions & 1 deletion compiler/src/dotty/tools/dotc/core/TypeOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import Contexts._, Types._, Symbols._, Names._, Flags._
import SymDenotations._
import util.Spans._
import util.Stats
import util.SourcePosition
import NameKinds.DepParamName
import Decorators._
import StdNames._
Expand Down
6 changes: 3 additions & 3 deletions compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class TreePickler(pickler: TastyPickler) {
// I believe it's a bug in typer: the type of an implicit argument refers
// to a closure parameter outside the closure itself. TODO: track this down, so that we
// can eliminate this case.
report.log(i"pickling reference to as yet undefined $sym in ${sym.owner}", sym.sourcePos)
report.log(i"pickling reference to as yet undefined $sym in ${sym.owner}", sym.srcPos)
pickleForwardSymRef(sym)
}

Expand Down Expand Up @@ -622,7 +622,7 @@ class TreePickler(pickler: TastyPickler) {
}
catch {
case ex: TypeError =>
report.error(ex.toMessage, tree.sourcePos.focus)
report.error(ex.toMessage, tree.srcPos.focus)
case ex: AssertionError =>
println(i"error when pickling tree $tree")
throw ex
Expand Down Expand Up @@ -734,7 +734,7 @@ class TreePickler(pickler: TastyPickler) {

def pickle(trees: List[Tree])(using Context): Unit = {
trees.foreach(tree => if (!tree.isEmpty) pickleTree(tree))
def missing = forwardSymRefs.keysIterator.map(sym => sym.showLocated + "(line " + sym.sourcePos.line + ")").toList
def missing = forwardSymRefs.keysIterator.map(sym => sym.showLocated + "(line " + sym.srcPos.line + ")").toList
assert(forwardSymRefs.isEmpty, i"unresolved symbols: $missing%, % when pickling ${ctx.source}")
}

Expand Down
Loading