-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix 1365: Fix bindings in patterns #1377
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -448,11 +448,12 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit | |
return typed(untpd.Apply(untpd.TypedSplice(arg), tree.expr), pt) | ||
case _ => | ||
} | ||
case tref: TypeRef if tref.symbol.isClass && !ctx.isAfterTyper => | ||
val setBefore = ctx.mode is Mode.GADTflexible | ||
tpt1.tpe.<:<(pt)(ctx.addMode(Mode.GADTflexible)) | ||
if (!setBefore) ctx.retractMode(Mode.GADTflexible) | ||
case _ => | ||
if (!ctx.isAfterTyper) { | ||
val setBefore = ctx.mode is Mode.GADTflexible | ||
tpt1.tpe.<:<(pt)(ctx.addMode(Mode.GADTflexible)) | ||
if (!setBefore) ctx.retractMode(Mode.GADTflexible) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you need to rectractMode? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is from the GADT fix, where I didn't want to change the state of the context apart from relaxing the solver for GADTs There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But as @DarkDimius notes it's a new context that gets created, so this should not be a problem. |
||
} | ||
} | ||
ascription(tpt1, isWildcard = true) | ||
} | ||
|
@@ -762,17 +763,37 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit | |
def typedCase(tree: untpd.CaseDef, pt: Type, selType: Type, gadtSyms: Set[Symbol])(implicit ctx: Context): CaseDef = track("typedCase") { | ||
val originalCtx = ctx | ||
|
||
def caseRest(pat: Tree)(implicit ctx: Context) = { | ||
pat foreachSubTree { | ||
case b: Bind => | ||
if (ctx.scope.lookup(b.name) == NoSymbol) ctx.enter(b.symbol) | ||
else ctx.error(d"duplicate pattern variable: ${b.name}", b.pos) | ||
case _ => | ||
/** - replace all references to symbols associated with wildcards by their GADT bounds | ||
* - enter all symbols introduced by a Bind in current scope | ||
*/ | ||
val indexPattern = new TreeMap { | ||
val elimWildcardSym = new TypeMap { | ||
def apply(t: Type) = t match { | ||
case ref @ TypeRef(_, tpnme.WILDCARD) if ctx.gadt.bounds.contains(ref.symbol) => | ||
ctx.gadt.bounds(ref.symbol) | ||
case TypeAlias(ref @ TypeRef(_, tpnme.WILDCARD)) if ctx.gadt.bounds.contains(ref.symbol) => | ||
ctx.gadt.bounds(ref.symbol) | ||
case _ => | ||
mapOver(t) | ||
} | ||
} | ||
override def transform(tree: Tree)(implicit ctx: Context) = | ||
super.transform(tree.withType(elimWildcardSym(tree.tpe))) match { | ||
case b: Bind => | ||
if (ctx.scope.lookup(b.name) == NoSymbol) ctx.enter(b.symbol) | ||
else ctx.error(d"duplicate pattern variable: ${b.name}", b.pos) | ||
b.symbol.info = elimWildcardSym(b.symbol.info) | ||
b | ||
case t => t | ||
} | ||
} | ||
|
||
def caseRest(pat: Tree)(implicit ctx: Context) = { | ||
val pat1 = indexPattern.transform(pat) | ||
val guard1 = typedExpr(tree.guard, defn.BooleanType) | ||
val body1 = ensureNoLocalRefs(typedExpr(tree.body, pt), pt, ctx.scope.toList) | ||
.ensureConforms(pt)(originalCtx) // insert a cast if body does not conform to expected type if we disregard gadt bounds | ||
assignType(cpy.CaseDef(tree)(pat, guard1, body1), body1) | ||
assignType(cpy.CaseDef(tree)(pat1, guard1, body1), body1) | ||
} | ||
|
||
val gadtCtx = | ||
|
@@ -963,11 +984,30 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit | |
assignType(cpy.ByNameTypeTree(tree)(result1), result1) | ||
} | ||
|
||
/** Define a new symbol associated with a Bind or pattern wildcard and | ||
* make it gadt narrowable. | ||
*/ | ||
private def newPatternBoundSym(name: Name, info: Type, pos: Position)(implicit ctx: Context) = { | ||
val flags = if (name.isTypeName) BindDefinedType else EmptyFlags | ||
val sym = ctx.newSymbol(ctx.owner, name, flags | Case, info, coord = pos) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
/** A case class or its companion object */
final val Case = commonFlag(17, "case")
final val CaseClass = Case.toTypeFlags
final val CaseVal = Case.toTermFlags so I don't understand what it's supposed to do here |
||
if (name.isTypeName) ctx.gadt.setBounds(sym, info.bounds) | ||
sym | ||
} | ||
|
||
def typedTypeBoundsTree(tree: untpd.TypeBoundsTree)(implicit ctx: Context): TypeBoundsTree = track("typedTypeBoundsTree") { | ||
val TypeBoundsTree(lo, hi) = desugar.typeBoundsTree(tree) | ||
val lo1 = typed(lo) | ||
val hi1 = typed(hi) | ||
assignType(cpy.TypeBoundsTree(tree)(lo1, hi1), lo1, hi1) | ||
val tree1 = assignType(cpy.TypeBoundsTree(tree)(lo1, hi1), lo1, hi1) | ||
if (ctx.mode.is(Mode.Pattern)) { | ||
// Associate a pattern-bound type symbol with the wildcard. | ||
// The bounds of the type symbol can be constrained when comparing a pattern type | ||
// with an expected type in typedTyped. The type symbol is eliminated once | ||
// the enclosing pattern has been typechecked; see `indexPattern` in `typedCase`. | ||
val wildcardSym = newPatternBoundSym(tpnme.WILDCARD, tree1.tpe, tree.pos) | ||
tree1.withType(wildcardSym.typeRef) | ||
} | ||
else tree1 | ||
} | ||
|
||
def typedBind(tree: untpd.Bind, pt: Type)(implicit ctx: Context): Tree = track("typedBind") { | ||
|
@@ -983,8 +1023,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit | |
tpd.cpy.UnApply(body1)(fn, Nil, | ||
typed(untpd.Bind(tree.name, arg).withPos(tree.pos), arg.tpe) :: Nil) | ||
case _ => | ||
val flags = if (tree.isType) BindDefinedType else EmptyFlags | ||
val sym = ctx.newSymbol(ctx.owner, tree.name, flags | Case, body1.tpe, coord = tree.pos) | ||
val sym = newPatternBoundSym(tree.name, body1.tpe, tree.pos) | ||
assignType(cpy.Bind(tree)(tree.name, body1), sym) | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import scala.collection.mutable.ArrayBuffer | ||
|
||
trait Message[M] | ||
class Script[S] extends ArrayBuffer[Message[S]] with Message[S] | ||
|
||
class Test[A] { | ||
def f(cmd: Message[A]): Unit = cmd match { | ||
case s: Script[_] => s.iterator.foreach(x => f(x)) | ||
} | ||
def g(cmd: Message[A]): Unit = cmd match { | ||
case s: Script[z] => s.iterator.foreach(x => g(x)) | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is equivalent but less hacky.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not describe it as hacky. The full code you propose would be:
It's certainly longer. There are quite a few other places in the code base by now that use the same idiom.