Skip to content

Check that dotty is reentrant #708

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 14 commits into from
Jul 6, 2015
Merged
2 changes: 1 addition & 1 deletion src/dotty/tools/dotc/Bench.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import reporting.Reporter

object Bench extends Driver {

private var numRuns = 1
@sharable private var numRuns = 1

def newCompiler(): Compiler = new Compiler

Expand Down
3 changes: 2 additions & 1 deletion src/dotty/tools/dotc/Compiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ class Compiler {
List(new FrontEnd),
List(new PostTyper),
List(new Pickler),
List(new FirstTransform),
List(new FirstTransform,
new CheckReentrant),
List(new RefChecks,
new ElimRepeated,
new NormalizeFlags,
Expand Down
2 changes: 1 addition & 1 deletion src/dotty/tools/dotc/Resident.scala
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import scala.annotation.tailrec
*
* dotc> :q // quit
*/
object Resident extends Driver {
class Resident extends Driver {

object residentCompiler extends Compiler

Expand Down
2 changes: 1 addition & 1 deletion src/dotty/tools/dotc/ast/Desugar.scala
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ object desugar {
else tdef
}

private val synthetic = Modifiers(Synthetic)
@sharable private val synthetic = Modifiers(Synthetic)

private def toDefParam(tparam: TypeDef): TypeDef =
tparam.withFlags(Param)
Expand Down
19 changes: 10 additions & 9 deletions src/dotty/tools/dotc/ast/Trees.scala
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
package dotty.tools.dotc
package dotty.tools
package dotc
package ast

import core._
Expand Down Expand Up @@ -26,7 +27,7 @@ object Trees {
type Untyped = Null

/** The total number of created tree nodes, maintained if Stats.enabled */
var ntrees = 0
@sharable var ntrees = 0

/** Modifiers and annotations for definitions
* @param flags The set flags
Expand Down Expand Up @@ -68,7 +69,7 @@ object Trees {
def tokenPos: Seq[(Token, Position)] = ???
}

private var nextId = 0 // for debugging
@sharable private var nextId = 0 // for debugging

type LazyTree = AnyRef /* really: Tree | Lazy[Tree] */
type LazyTreeList = AnyRef /* really: List[Tree] | Lazy[List[Tree]] */
Expand Down Expand Up @@ -723,9 +724,9 @@ object Trees {
setMods(Modifiers[T](PrivateLocal))
}

val theEmptyTree: Thicket[Type] = Thicket(Nil)
val theEmptyValDef = new EmptyValDef[Type]
val theEmptyModifiers = new Modifiers()
@sharable val theEmptyTree: Thicket[Type] = Thicket(Nil)
@sharable val theEmptyValDef = new EmptyValDef[Type]
@sharable val theEmptyModifiers = new Modifiers()

def genericEmptyValDef[T >: Untyped]: ValDef[T] = theEmptyValDef.asInstanceOf[ValDef[T]]
def genericEmptyTree[T >: Untyped]: Thicket[T] = theEmptyTree.asInstanceOf[Thicket[T]]
Expand Down Expand Up @@ -845,9 +846,9 @@ object Trees {
type Annotated = Trees.Annotated[T]
type Thicket = Trees.Thicket[T]

val EmptyTree: Thicket = genericEmptyTree
val EmptyValDef: ValDef = genericEmptyValDef
val EmptyModifiers: Modifiers = genericEmptyModifiers
@sharable val EmptyTree: Thicket = genericEmptyTree
@sharable val EmptyValDef: ValDef = genericEmptyValDef
@sharable val EmptyModifiers: Modifiers = genericEmptyModifiers

// ----- Auxiliary creation methods ------------------

Expand Down
4 changes: 2 additions & 2 deletions src/dotty/tools/dotc/config/Properties.scala
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ object Properties extends PropertiesTrait {

/** Scala manifest attributes.
*/
val ScalaCompilerVersion = new AttributeName("Scala-Compiler-Version")
@sharable val ScalaCompilerVersion = new AttributeName("Scala-Compiler-Version")
}

trait PropertiesTrait {
Expand All @@ -23,7 +23,7 @@ trait PropertiesTrait {
protected val propFilename = "/" + propCategory + ".properties"

/** The loaded properties */
protected lazy val scalaProps: java.util.Properties = {
@sharable protected lazy val scalaProps: java.util.Properties = {
val props = new java.util.Properties
val stream = pickJarBasedOn getResourceAsStream propFilename
if (stream ne null)
Expand Down
1 change: 1 addition & 0 deletions src/dotty/tools/dotc/config/ScalaSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ class ScalaSettings extends Settings.SettingGroup {
val YnoDeepSubtypes = BooleanSetting("-Yno-deep-subtypes", "throw an exception on deep subtyping call stacks.")
val YprintSyms = BooleanSetting("-Yprint-syms", "when printing trees print info in symbols instead of corresponding info in trees.")
val YtestPickler = BooleanSetting("-Ytest-pickler", "self-test for pickling functionality; should be used with -Ystop-after:pickler")
val YcheckReentrant = BooleanSetting("-Ycheck-reentrant", "check that compiled program does not contain vars that can be accessed from a global root.")
def stop = YstopAfter

/** Area-specific debug output.
Expand Down
9 changes: 5 additions & 4 deletions src/dotty/tools/dotc/config/ScalaVersion.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* @author James Iry
*/
package dotty.tools.dotc.config
package dotty.tools
package dotc.config

import scala.util.{Try, Success, Failure}

Expand All @@ -15,7 +16,7 @@ sealed abstract class ScalaVersion extends Ordered[ScalaVersion] {
/**
* A scala version that sorts higher than all actual versions
*/
case object NoScalaVersion extends ScalaVersion {
@sharable case object NoScalaVersion extends ScalaVersion {
def unparse = "none"

def compare(that: ScalaVersion): Int = that match {
Expand Down Expand Up @@ -52,7 +53,7 @@ case class SpecificScalaVersion(major: Int, minor: Int, rev: Int, build: ScalaBu
/**
* A Scala version that sorts lower than all actual versions
*/
case object AnyScalaVersion extends ScalaVersion {
@sharable case object AnyScalaVersion extends ScalaVersion {
def unparse = "any"

def compare(that: ScalaVersion): Int = that match {
Expand All @@ -64,7 +65,7 @@ case object AnyScalaVersion extends ScalaVersion {
/**
* Methods for parsing ScalaVersions
*/
object ScalaVersion {
@sharable object ScalaVersion {
private val dot = "\\."
private val dash = "\\-"
private def not(s:String) = s"[^${s}]"
Expand Down
5 changes: 3 additions & 2 deletions src/dotty/tools/dotc/core/Contexts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import printing._
import config.{Settings, ScalaSettings, Platform, JavaPlatform}
import language.implicitConversions
import DenotTransformers.DenotTransformer

object Contexts {

/** A context is passed basically everywhere in dotc.
Expand Down Expand Up @@ -473,7 +474,7 @@ object Contexts {
gadt = new GADTMap(SimpleMap.Empty)
}

object NoContext extends Context {
@sharable object NoContext extends Context {
lazy val base = unsupported("base")
override val implicits: ContextualImplicits = new ContextualImplicits(Nil, null)(this)
}
Expand Down Expand Up @@ -620,7 +621,7 @@ object Contexts {
/** implicit conversion that injects all ContextBase members into a context */
implicit def toBase(ctx: Context): ContextBase = ctx.base

val theBase = new ContextBase // !!! DEBUG, so that we can use a minimal context for reporting even in code that normally cannot access a context
// @sharable val theBase = new ContextBase // !!! DEBUG, so that we can use a minimal context for reporting even in code that normally cannot access a context
}

/** Info that changes on each compiler run */
Expand Down
6 changes: 4 additions & 2 deletions src/dotty/tools/dotc/core/Denotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ object Denotations {
*/
private def bringForward()(implicit ctx: Context): SingleDenotation = this match {
case denot: SymDenotation if ctx.stillValid(denot) =>
if (denot.exists) assert(ctx.runId > validFor.runId, s"denotation $denot invalid in run ${ctx.runId}. ValidFor: $validFor")
assert(ctx.runId > validFor.runId, s"denotation $denot invalid in run ${ctx.runId}. ValidFor: $validFor")
var d: SingleDenotation = denot
do {
d.validFor = Period(ctx.period.runId, d.validFor.firstPhaseId, d.validFor.lastPhaseId)
Expand Down Expand Up @@ -592,7 +592,9 @@ object Denotations {
assert(false)
}

if (valid.runId != currentPeriod.runId) initial.bringForward.current
if (valid.runId != currentPeriod.runId)
if (exists) initial.bringForward.current
else this
else {
var cur = this
if (currentPeriod.code > valid.code) {
Expand Down
2 changes: 1 addition & 1 deletion src/dotty/tools/dotc/core/Flags.scala
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ object Flags {
private final val FirstNotPickledFlag = 48
private final val MaxFlag = 63

private var flagName = Array.fill(64, 2)("")
private val flagName = Array.fill(64, 2)("")

private def isDefinedAsFlag(idx: Int) = flagName(idx) exists (_.nonEmpty)

Expand Down
91 changes: 50 additions & 41 deletions src/dotty/tools/dotc/core/Names.scala
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
package dotty.tools.dotc
package dotty.tools
package dotc
package core

import scala.io.Codec
Expand Down Expand Up @@ -151,11 +152,13 @@ object Names {
override def seq = toCollection(this)
}

class TermName(val start: Int, val length: Int, private[Names] var next: TermName) extends Name {
class TermName(val start: Int, val length: Int, @sharable private[Names] var next: TermName) extends Name {
// `next` is @sharable because it is only modified in the synchronized block of termName.
type ThisName = TermName
def isTypeName = false
def isTermName = true

@sharable // because it is only modified in the synchronized block of toTypeName.
@volatile private[this] var _typeName: TypeName = null

def toTypeName: TypeName = {
Expand Down Expand Up @@ -200,44 +203,21 @@ object Names {
private final val fillFactor = 0.7

/** Memory to store all names sequentially. */
@sharable // because it's only mutated in synchronized block of termName
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping track and maintaining such annotation could be a burden.
Instead of annotating explicitly, could we infer that private fields that are modified in synchronized(this) blocks are sharable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe at some point. But right now that's probably too much of a detour. Let's see first whether @sharable thing becomes a feature of general utility. Even now it serves a purpose: If we want to figure out data races, it's good to scrutinize members with @sharable annotations. For the others we can run -Ycheck:reentrant to rule them out (well, almost, modulo to the shortcomings mentioned in the comment).

private[dotty] var chrs: Array[Char] = new Array[Char](InitialNameSize)

/** The number of characters filled. */
@sharable // because it's only mutated in synchronized block of termName
private var nc = 0

/** Hashtable for finding term names quickly. */
@sharable // because it's only mutated in synchronized block of termName
private var table = new Array[TermName](InitialHashSize)

/** The number of defined names. */
@sharable // because it's only mutated in synchronized block of termName
private var size = 1

/** Make sure the capacity of the character array is at least `n` */
private def ensureCapacity(n: Int) =
if (n > chrs.length) {
val newchrs = new Array[Char](chrs.length * 2)
chrs.copyToArray(newchrs)
chrs = newchrs
}

/** Make sure the hash table is large enough for the given load factor */
private def incTableSize() = {
size += 1
if (size.toDouble / table.size > fillFactor) {
val oldTable = table
table = new Array[TermName](table.size * 2)
for (i <- 0 until oldTable.size) rehash(oldTable(i))
}
}

/** Rehash chain of names */
private def rehash(name: TermName): Unit =
if (name != null) {
rehash(name.next)
val h = hashValue(chrs, name.start, name.length) & (table.size - 1)
name.next = table(h)
table(h) = name
}

/** The hash of a name made of from characters cs[offset..offset+len-1]. */
private def hashValue(cs: Array[Char], offset: Int, len: Int): Int =
if (len > 0)
Expand All @@ -257,24 +237,53 @@ object Names {
i == len
}

/** Enter characters into chrs array. */
private def enterChars(cs: Array[Char], offset: Int, len: Int): Unit = {
ensureCapacity(nc + len)
var i = 0
while (i < len) {
chrs(nc + i) = cs(offset + i)
i += 1
}
nc += len
}

/** Create a term name from the characters in cs[offset..offset+len-1].
* Assume they are already encoded.
*/
def termName(cs: Array[Char], offset: Int, len: Int): TermName = {
util.Stats.record("termName")
val h = hashValue(cs, offset, len) & (table.size - 1)

synchronized {

/** Make sure the capacity of the character array is at least `n` */
def ensureCapacity(n: Int) =
if (n > chrs.length) {
val newchrs = new Array[Char](chrs.length * 2)
chrs.copyToArray(newchrs)
chrs = newchrs
}

/** Enter characters into chrs array. */
def enterChars(): Unit = {
ensureCapacity(nc + len)
var i = 0
while (i < len) {
chrs(nc + i) = cs(offset + i)
i += 1
}
nc += len
}

/** Rehash chain of names */
def rehash(name: TermName): Unit =
if (name != null) {
rehash(name.next)
val h = hashValue(chrs, name.start, name.length) & (table.size - 1)
name.next = table(h)
table(h) = name
}

/** Make sure the hash table is large enough for the given load factor */
def incTableSize() = {
size += 1
if (size.toDouble / table.size > fillFactor) {
val oldTable = table
table = new Array[TermName](table.size * 2)
for (i <- 0 until oldTable.size) rehash(oldTable(i))
}
}

val next = table(h)
var name = next
while (name ne null) {
Expand All @@ -283,7 +292,7 @@ object Names {
name = name.next
}
name = new TermName(nc, len, next)
enterChars(cs, offset, len)
enterChars()
table(h) = name
incTableSize()
name
Expand Down
9 changes: 5 additions & 4 deletions src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
package dotty.tools.dotc
package dotty.tools
package dotc
package core

import Periods._, Contexts._, Symbols._, Denotations._, Names._, NameOps._, Annotations._
Expand Down Expand Up @@ -1713,8 +1714,8 @@ object SymDenotations {
validFor = Period.allInRun(NoRunId) // will be brought forward automatically
}

val NoDenotation = new NoDenotation
val NotDefinedHereDenotation = new NoDenotation
@sharable val NoDenotation = new NoDenotation
@sharable val NotDefinedHereDenotation = new NoDenotation

// ---- Completion --------------------------------------------------------

Expand Down Expand Up @@ -1757,7 +1758,7 @@ object SymDenotations {
val NoSymbolFn = (ctx: Context) => NoSymbol

/** A missing completer */
class NoCompleter extends LazyType {
@sharable class NoCompleter extends LazyType {
def complete(denot: SymDenotation)(implicit ctx: Context): Unit = unsupported("complete")
}

Expand Down
Loading