Skip to content

Fix #825 #834

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 7 commits into from
Oct 22, 2015
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
20 changes: 12 additions & 8 deletions src/dotty/tools/dotc/ast/Desugar.scala
Original file line number Diff line number Diff line change
Expand Up @@ -380,18 +380,22 @@ object desugar {
// synthetic implicit C[Ts](p11: T11, ..., p1N: T1N) ... (pM1: TM1, ..., pMN: TMN): C[Ts] =
// new C[Ts](p11, ..., p1N) ... (pM1, ..., pMN) =
val implicitWrappers =
if (mods is Implicit) {
if (ctx.owner is Package)
ctx.error("implicit classes may not be toplevel", cdef.pos)
if (mods is Case)
ctx.error("implicit classes may not case classes", cdef.pos)

if (!mods.is(Implicit))
Nil
else if (ctx.owner is Package) {
ctx.error("implicit classes may not be toplevel", cdef.pos)
Nil
}
else if (mods is Case) {
Copy link
Member

Choose a reason for hiding this comment

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

I think our coding style is } else if ... without a line break after the }.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No I write newline } quite systematically now. Makes keywords align better.

On Wed, Oct 21, 2015 at 6:43 PM, Guillaume Martres <[email protected]

wrote:

In src/dotty/tools/dotc/ast/Desugar.scala
#834 (comment):

@@ -380,18 +380,22 @@ object desugar {
// synthetic implicit C[Ts](p11: T11, ..., p1N: T1N) ... (pM1: TM1, ..., pMN: TMN): C[Ts] =
// new C[Ts](p11, ..., p1N) ... (pM1, ..., pMN) =
val implicitWrappers =

  •  if (mods is Implicit) {
    
  •    if (ctx.owner is Package)
    
  •      ctx.error("implicit classes may not be toplevel", cdef.pos)
    
  •    if (mods is Case)
    

- ctx.error("implicit classes may not case classes", cdef.pos)

  •  if (!mods.is(Implicit))
    
  •    Nil
    
  •  else if (ctx.owner is Package) {
    
  •    ctx.error("implicit classes may not be toplevel", cdef.pos)
    
  •    Nil
    
  •  }
    
  •  else if (mods is Case) {
    

I think our coding style is } else if ... without a line break after the }
.


Reply to this email directly or view it on GitHub
https://github.com/lampepfl/dotty/pull/834/files#r42649286.

Martin Odersky
EPFL

ctx.error("implicit classes may not be case classes", cdef.pos)
Nil
}
else
// implicit wrapper is typechecked in same scope as constructor, so
// we can reuse the constructor parameters; no derived params are needed.
DefDef(name.toTermName, constrTparams, constrVparamss, classTypeRef, creatorExpr)
.withFlags(Synthetic | Implicit) :: Nil
}
else Nil


val self1 = {
val selfType = if (self.tpt.isEmpty) classTypeRef else self.tpt
Expand Down
75 changes: 42 additions & 33 deletions src/dotty/tools/dotc/core/TypeErasure.scala
Original file line number Diff line number Diff line change
Expand Up @@ -401,10 +401,10 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean
private def eraseDerivedValueClassRef(tref: TypeRef)(implicit ctx: Context): Type = {
val cls = tref.symbol.asClass
val underlying = underlyingOfValueClass(cls)
ErasedValueType(cls, valueErasure(underlying))
if (underlying.exists) ErasedValueType(cls, valueErasure(underlying))
else NoType
}


private def eraseNormalClassRef(tref: TypeRef)(implicit ctx: Context): Type = {
val cls = tref.symbol.asClass
(if (cls.owner is Package) normalizeClass(cls) else cls).typeRef
Expand Down Expand Up @@ -439,36 +439,45 @@ class TypeErasure(isJava: Boolean, semiEraseVCs: Boolean, isConstructor: Boolean
/** The name of the type as it is used in `Signature`s.
* Need to ensure correspondence with erasure!
*/
private def sigName(tp: Type)(implicit ctx: Context): TypeName = tp match {
case ErasedValueType(_, underlying) =>
sigName(underlying)
case tp: TypeRef =>
if (!tp.denot.exists) throw new MissingType(tp.prefix, tp.name)
val sym = tp.symbol
if (!sym.isClass) {
val info = tp.info
if (!info.exists) assert(false, "undefined: $tp with symbol $sym")
sigName(info)
}
else if (isDerivedValueClass(sym)) sigName(eraseDerivedValueClassRef(tp))
else normalizeClass(sym.asClass).fullName.asTypeName
case defn.ArrayType(elem) =>
sigName(this(tp))
case JavaArrayType(elem) =>
sigName(elem) ++ "[]"
case tp: TermRef =>
sigName(tp.widen)
case ExprType(rt) =>
sigName(defn.FunctionType(Nil, rt))
case tp: TypeProxy =>
sigName(tp.underlying)
case ErrorType | WildcardType =>
tpnme.WILDCARD
case tp: WildcardType =>
sigName(tp.optBounds)
case _ =>
val erased = this(tp)
assert(erased ne tp, tp)
sigName(erased)
private def sigName(tp: Type)(implicit ctx: Context): TypeName = try {
tp match {
case ErasedValueType(_, underlying) =>
sigName(underlying)
case tp: TypeRef =>
if (!tp.denot.exists) throw new MissingType(tp.prefix, tp.name)
val sym = tp.symbol
if (!sym.isClass) {
val info = tp.info
if (!info.exists) assert(false, "undefined: $tp with symbol $sym")
return sigName(info)
}
if (isDerivedValueClass(sym)) {
val erasedVCRef = eraseDerivedValueClassRef(tp)
if (erasedVCRef.exists) return sigName(erasedVCRef)
}
normalizeClass(sym.asClass).fullName.asTypeName
case defn.ArrayType(elem) =>
sigName(this(tp))
case JavaArrayType(elem) =>
sigName(elem) ++ "[]"
case tp: TermRef =>
sigName(tp.widen)
case ExprType(rt) =>
sigName(defn.FunctionType(Nil, rt))
case tp: TypeProxy =>
sigName(tp.underlying)
case ErrorType | WildcardType =>
tpnme.WILDCARD
case tp: WildcardType =>
sigName(tp.optBounds)
case _ =>
val erased = this(tp)
assert(erased ne tp, tp)
sigName(erased)
}
} catch {
case ex: AssertionError =>
println(s"no sig for $tp")
throw ex
}
}
28 changes: 19 additions & 9 deletions src/dotty/tools/dotc/parsing/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1343,7 +1343,7 @@ object Parsers {
private def flagOfToken(tok: Int): FlagSet = tok match {
case ABSTRACT => Abstract
case FINAL => Final
case IMPLICIT => Implicit
case IMPLICIT => ImplicitCommon
case LAZY => Lazy
case OVERRIDE => Override
case PRIVATE => Private
Expand Down Expand Up @@ -1377,12 +1377,21 @@ object Parsers {
|| flags1.isTypeFlags && flags2.isTypeFlags
)

def addFlag(mods: Modifiers, flag: FlagSet): Modifiers =
def addFlag(mods: Modifiers, flag: FlagSet): Modifiers = {
def incompatible(kind: String) = {
syntaxError(s"modifier(s) `${mods.flags}' not allowed for $kind")
Modifiers(flag)
}
if (compatible(mods.flags, flag)) mods | flag
else {
syntaxError(s"illegal modifier combination: ${mods.flags} and $flag")
mods
else flag match {
case Trait => incompatible("trait")
case Method => incompatible("method")
case Mutable => incompatible("variable")
case _ =>
syntaxError(s"illegal modifier combination: ${mods.flags} and $flag")
mods
}
}

/** AccessQualifier ::= "[" (Id | this) "]"
*/
Expand Down Expand Up @@ -1726,6 +1735,7 @@ object Parsers {
}
makeConstructor(Nil, vparamss, rhs).withMods(mods)
} else {
val mods1 = addFlag(mods, Method)
val name = ident()
val tparams = typeParamClauseOpt(ParamOwner.Def)
val vparamss = paramClauses(name)
Expand All @@ -1735,7 +1745,7 @@ object Parsers {
if (atScala2Brace) tpt = scalaUnit else accept(EQUALS)
expr()
} else EmptyTree
DefDef(name, tparams, vparamss, tpt, rhs).withMods(mods | Method)
DefDef(name, tparams, vparamss, tpt, rhs).withMods(mods1)
}
}

Expand Down Expand Up @@ -1792,7 +1802,7 @@ object Parsers {
*/
def tmplDef(start: Int, mods: Modifiers): Tree = in.token match {
case TRAIT =>
classDef(posMods(start, mods | Trait))
classDef(posMods(start, addFlag(mods, Trait)))
case CLASS =>
classDef(posMods(start, mods))
case CASECLASS =>
Expand Down Expand Up @@ -2009,7 +2019,7 @@ object Parsers {
}

def localDef(start: Int, implicitFlag: FlagSet): Tree =
defOrDcl(start, defAnnotsMods(localModifierTokens) | implicitFlag)
defOrDcl(start, addFlag(defAnnotsMods(localModifierTokens), implicitFlag))

/** BlockStatSeq ::= { BlockStat semi } [ResultExpr]
* BlockStat ::= Import
Expand Down Expand Up @@ -2038,7 +2048,7 @@ object Parsers {
if (in.token == IMPLICIT) {
val start = in.skipToken()
if (isIdent) stats += implicitClosure(start, Location.InBlock)
else stats += localDef(start, Implicit)
else stats += localDef(start, ImplicitCommon)
} else {
stats += localDef(in.offset, EmptyFlags)
}
Expand Down
44 changes: 44 additions & 0 deletions src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,50 @@ object Checking {
}
checkTree((), refinement)
}

/** Check that symbol's definition is well-formed. */
def checkWellFormed(sym: Symbol)(implicit ctx: Context): Unit = {
//println(i"check wf $sym with flags ${sym.flags}")
def fail(msg: String) = ctx.error(msg, sym.pos)
def varNote =
if (sym.is(Mutable)) "\n(Note that variables need to be initialized to be defined)"
else ""

def checkWithDeferred(flag: FlagSet) =
if (sym.is(flag))
fail(i"abstract member may not have `$flag' modifier")
def checkNoConflict(flag1: FlagSet, flag2: FlagSet) =
if (sym.is(allOf(flag1, flag2)))
fail(i"illegal combination of modifiers: $flag1 and $flag2 for: $sym")

if (sym.is(ImplicitCommon)) {
if (sym.owner.is(Package))
fail(i"`implicit' modifier cannot be used for top-level definitions")
if (sym.isType)
fail(i"`implicit' modifier cannot be used for types or traits")
}
if (!sym.isClass && sym.is(Abstract))
fail(i"`abstract' modifier can be used only for classes; it should be omitted for abstract members")
if (sym.is(AbsOverride) && !sym.owner.is(Trait))
fail(i"`abstract override' modifier only allowed for members of traits")
if (sym.is(Trait) && sym.is(Final))
fail(i"$sym may not be `final'")
if (sym.hasAnnotation(defn.NativeAnnot)) {
if (!sym.is(Deferred))
fail(i"`@native' members may not have implementation")
}
else if (sym.is(Deferred, butNot = Param) && !sym.isSelfSym) {
if (!sym.owner.isClass || sym.owner.is(Module) || sym.owner.isAnonymousClass)
fail(i"only classes can have declared but undefined members$varNote")
checkWithDeferred(Private)
checkWithDeferred(Final)
}
if (sym.isValueClass && sym.is(Trait) && !sym.isRefinementClass)
fail(i"$sym cannot extend AnyVal")
checkNoConflict(Final, Sealed)
checkNoConflict(Private, Protected)
checkNoConflict(Abstract, Override)
}
}

trait Checking {
Expand Down
20 changes: 18 additions & 2 deletions src/dotty/tools/dotc/typer/Namer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,18 @@ class Namer { typer: Typer =>
sym
}

def checkFlags(flags: FlagSet) =
if (flags.isEmpty) flags
else {
val (ok, adapted, kind) = tree match {
case tree: TypeDef => (flags.isTypeFlags, flags.toTypeFlags, "type")
case _ => (flags.isTermFlags, flags.toTermFlags, "value")
}
if (!ok)
ctx.error(i"modifier(s) `$flags' incompatible with $kind definition", tree.pos)
adapted
}

/** Add moduleClass/sourceModule to completer if it is for a module val or class */
def adjustIfModule(completer: LazyType, tree: MemberDef) =
if (tree.mods is Module) ctx.adjustModuleCompleter(completer, tree.name.encode)
Expand Down Expand Up @@ -261,14 +273,16 @@ class Namer { typer: Typer =>
tree match {
case tree: TypeDef if tree.isClassDef =>
val name = checkNoConflict(tree.name.encode).asTypeName
val flags = checkFlags(tree.mods.flags &~ Implicit)
val cls = record(ctx.newClassSymbol(
ctx.owner, name, tree.mods.flags | inSuperCall,
ctx.owner, name, flags | inSuperCall,
cls => adjustIfModule(new ClassCompleter(cls, tree)(ctx), tree),
privateWithinClass(tree.mods), tree.pos, ctx.source.file))
cls.completer.asInstanceOf[ClassCompleter].init()
cls
case tree: MemberDef =>
val name = checkNoConflict(tree.name.encode)
val flags = checkFlags(tree.mods.flags)
val isDeferred = lacksDefinition(tree)
val deferred = if (isDeferred) Deferred else EmptyFlags
val method = if (tree.isInstanceOf[DefDef]) Method else EmptyFlags
Expand All @@ -291,7 +305,7 @@ class Namer { typer: Typer =>
val cctx = if (tree.name == nme.CONSTRUCTOR && !(tree.mods is JavaDefined)) ctx.outer else ctx

record(ctx.newSymbol(
ctx.owner, name, tree.mods.flags | deferred | method | higherKinded | inSuperCall1,
ctx.owner, name, flags | deferred | method | higherKinded | inSuperCall1,
adjustIfModule(new Completer(tree)(cctx), tree),
privateWithinClass(tree.mods), tree.pos))
case tree: Import =>
Expand Down Expand Up @@ -517,6 +531,7 @@ class Namer { typer: Typer =>
def completeInCreationContext(denot: SymDenotation): Unit = {
denot.info = typeSig(denot.symbol)
addAnnotations(denot)
Checking.checkWellFormed(denot.symbol)
}
}

Expand Down Expand Up @@ -585,6 +600,7 @@ class Namer { typer: Typer =>
index(rest)(inClassContext(selfInfo))
denot.info = ClassInfo(cls.owner.thisType, cls, parentRefs, decls, selfInfo)
addAnnotations(denot)
Checking.checkWellFormed(cls)
if (isDerivedValueClass(cls)) cls.setFlag(Final)
cls.setApplicableFlags(
(NoInitsInterface /: impl.body)((fs, stat) => fs & defKind(stat)))
Expand Down
9 changes: 6 additions & 3 deletions test/dotc/tests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class tests extends CompilerTest {
@Test def neg_typedapply() = compileFile(negDir, "typedapply", xerrors = 4)
@Test def neg_typedidents() = compileFile(negDir, "typedIdents", xerrors = 2)
@Test def neg_assignments() = compileFile(negDir, "assignments", xerrors = 3)
@Test def neg_typers() = compileFile(negDir, "typers", xerrors = 13)(allowDoubleBindings)
@Test def neg_typers() = compileFile(negDir, "typers", xerrors = 14)(allowDoubleBindings)
@Test def neg_privates() = compileFile(negDir, "privates", xerrors = 2)
@Test def neg_rootImports = compileFile(negDir, "rootImplicits", xerrors = 2)
@Test def neg_templateParents() = compileFile(negDir, "templateParents", xerrors = 3)
Expand Down Expand Up @@ -135,7 +135,7 @@ class tests extends CompilerTest {
@Test def neg_badAuxConstr = compileFile(negDir, "badAuxConstr", xerrors = 2)
@Test def neg_typetest = compileFile(negDir, "typetest", xerrors = 1)
@Test def neg_t1569_failedAvoid = compileFile(negDir, "t1569-failedAvoid", xerrors = 1)
@Test def neg_clashes = compileFile(negDir, "clashes", xerrors = 2)
@Test def neg_clashes = compileFile(negDir, "clashes", xerrors = 3)
@Test def neg_cycles = compileFile(negDir, "cycles", xerrors = 8)
@Test def neg_boundspropagation = compileFile(negDir, "boundspropagation", xerrors = 5)
@Test def neg_refinedSubtyping = compileFile(negDir, "refinedSubtyping", xerrors = 2)
Expand All @@ -150,13 +150,16 @@ class tests extends CompilerTest {
@Test def neg_instantiateAbstract = compileFile(negDir, "instantiateAbstract", xerrors = 8)
@Test def neg_selfInheritance = compileFile(negDir, "selfInheritance", xerrors = 6)
@Test def neg_selfreq = compileFile(negDir, "selfreq", xerrors = 4)
@Test def neg_singletons = compileFile(negDir, "singletons", xerrors = 5)
@Test def neg_singletons = compileFile(negDir, "singletons", xerrors = 8)
@Test def neg_shadowedImplicits = compileFile(negDir, "arrayclone-new", xerrors = 2)
@Test def neg_traitParamsTyper = compileFile(negDir, "traitParamsTyper", xerrors = 5)
@Test def neg_traitParamsMixin = compileFile(negDir, "traitParamsMixin", xerrors = 2)
@Test def neg_firstError = compileFile(negDir, "firstError", xerrors = 3)
@Test def neg_implicitLowerBound = compileFile(negDir, "implicit-lower-bound", xerrors = 1)
@Test def neg_partialApplications = compileFile(negDir, "partialApplications", xerrors = 8)
@Test def neg_validate = compileFile(negDir, "validate", xerrors = 18)
@Test def neg_validateParsing = compileFile(negDir, "validate-parsing", xerrors = 7)
@Test def neg_validateRefchecks = compileFile(negDir, "validate-refchecks", xerrors = 2)

@Test def run_all = runFiles(runDir)

Expand Down
2 changes: 1 addition & 1 deletion tests/neg/i50-volatile.scala
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
object Test {
class Test {
class Base {
class Inner
}
Expand Down
2 changes: 1 addition & 1 deletion tests/neg/singletons.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ object Test {

val n: null = null // error: Null is not a legal singleton type

val sym: 'sym = 'sym // error: Symbol is a legal singleton type
val sym: 'sym = 'sym // error: Symbol is not a legal singleton type

val foo: s"abc" = "abc" // error: not a legal singleton type
}
2 changes: 1 addition & 1 deletion tests/neg/typers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ object typers {
}

type L[X] = scala.collection.immutable.List[X]
type M[X, Y] <: scala.collection.immutable.Map[X, Y]
type M[X, Y] <: scala.collection.immutable.Map[X, Y] // error: only classes can have declared but undefined members

object hk {
def f(x: L) // error: missing type parameter
Expand Down
13 changes: 13 additions & 0 deletions tests/neg/validate-parsing.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
object A {
sealed def y: Int = 1 // error: modifier(s) `sealed' not allowed for method
sealed var x = 1 // error: modifier(s) `sealed' not allowed for variable
lazy trait T // error: modifier(s) `lazy' not allowed for trait
}

class C () {
implicit this() = this() // error: ';' expected but 'implicit' found.
override this() = this() // error: ';' expected but 'override' found.
}
class D override() // error: ';' expected but 'override' found.

case class ByName(x: => Int) // error: `val' parameters may not be call-by-name
Loading