-
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
Conversation
We need to compare pattern types with expected types in order to derive knowledge about pattern-bound variables. This is done use the mechanism of gadt bounds.
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 comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need to rectractMode? ctx
here is not fresh and addMode
should create a new one instead of mutating existing one.
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.
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 comment
The 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.
Otherwise LGTM |
It would be also nice to add a test. |
The dropped lines were both pure functions whose result is ignored.
@DarkDimius There is a test: pos/1365.scala. Should something else be added to it? |
*/ | ||
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 comment
The reason will be displayed to describe this comment to others. Learn more.
Case
is described as:
/** 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
We need to compare pattern types with expected types in order
to derive knowledge about pattern-bound variables. This is done
using the mechanism of gadt bounds.
This is a subtle aspect of type inference that was so far missing from dotty. scalac does it
differently, I would argue in a more ad-hoc way.
Review by @DarkDimius.