Skip to content

Implement multiple assignments (SIP-59) #19597

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

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
e4682ce
Add a test for multiple assignments
kyouko-taiga Jan 30, 2024
af187a5
Allow tuple expressions on the LHS of assignments
kyouko-taiga Jan 30, 2024
99bcde3
Strengthen tests for multiple assignments
kyouko-taiga Feb 2, 2024
d0e46b5
Add error messages for type errors in multiple assignment
kyouko-taiga Feb 2, 2024
716a323
Handle multiple assignments in the typer
kyouko-taiga Feb 2, 2024
1664620
Add the Fibonacci sequence example from SIP-59 to the tests
kyouko-taiga Feb 2, 2024
ed92d95
Use errorTree rather than pruning ill-typed multiple assignments
kyouko-taiga Feb 12, 2024
6b4b3f0
Form a single block with all assignments
kyouko-taiga Feb 12, 2024
a9d11fd
Replace tailrec def with a for loop
kyouko-taiga Feb 12, 2024
f414f67
Refactor 'typedAssign' to ensure the right evaluation order
kyouko-taiga Jun 5, 2024
431cffd
Update compiler/src/dotty/tools/dotc/typer/PartialAssignment.scala
kyouko-taiga Jun 5, 2024
4566328
Use unadapted apply in desugaring of setter
kyouko-taiga Jun 18, 2024
1b98ba4
Fix the computation of partial assignments to setter in extensions
kyouko-taiga Jul 25, 2024
71fbbd0
Remove needless parameter
kyouko-taiga Jul 25, 2024
9a2d8a6
Improve choice of words in comments
kyouko-taiga Jul 25, 2024
2841e53
Remove needless filter
kyouko-taiga Jul 25, 2024
3b83559
Refactor expression to work around an inference regression
kyouko-taiga Jul 25, 2024
c2a3705
Update test expectations
kyouko-taiga Jul 25, 2024
6cf4e15
Remember the span of hoisted values
kyouko-taiga Jul 26, 2024
e16dcde
Fix the hoisting of multiple-assignment targets
kyouko-taiga Jul 26, 2024
833d329
Add missing spans
kyouko-taiga Jul 26, 2024
65c428b
Add missing comment
kyouko-taiga Jul 26, 2024
5dab0f6
Add evaluation tests for multiple assignments
kyouko-taiga Jul 26, 2024
51e863f
Fix setters desugaring
kyouko-taiga Jul 26, 2024
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
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/parsing/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2479,7 +2479,7 @@ object Parsers {
def expr1Rest(t: Tree, location: Location): Tree =
if in.token == EQUALS then
t match
case Ident(_) | Select(_, _) | Apply(_, _) | PrefixOp(_, _) =>
case Ident(_) | Select(_, _) | Apply(_, _) | PrefixOp(_, _) | Tuple(_) =>
atSpan(startOffset(t), in.skipToken()) {
val loc = if location.inArgs then location else Location.ElseWhere
Assign(t, subPart(() => expr(loc)))
Expand Down
3 changes: 3 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,9 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
case UnusedSymbolID // errorNumber: 198
case TailrecNestedCallID //errorNumber: 199
case FinalLocalDefID // errorNumber: 200
case InvalidMultipleAssignmentSourceID // errorNumber: 201
case InvalidMultipleAssignmentTargetID // errorNumber: 202
case MultipleAssignmentShapeMismatchID // errorNumber: 203

def errorNumber = ordinal - 1

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@ trait UniqueMessagePositions extends Reporter {
for offset <- dia.pos.start to dia.pos.end do
positions.get((ctx.source, offset)) match
case Some(dia1) if dia1.hides(dia) =>
case _ => positions((ctx.source, offset)) = dia
case _ => positions((ctx.source, Integer.valueOf(offset).nn)) = dia
Copy link
Member

Choose a reason for hiding this comment

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

Is this change required? If yes, why?

super.markReported(dia)
}
25 changes: 25 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3288,3 +3288,28 @@ object UnusedSymbol {
def privateMembers(using Context): UnusedSymbol = new UnusedSymbol(i"unused private member")
def patVars(using Context): UnusedSymbol = new UnusedSymbol(i"unused pattern variable")
}

class InvalidMultipleAssignmentSource(found: Type)(using Context)
extends TypeMsg(InvalidMultipleAssignmentSourceID) {
def msg(using Context) =
i"""invalid source of multiple assignment.
|The right hand side must be a tuple but $found was found."""
def explain(using Context) = ""
}

class InvalidMultipleAssignmentTarget()(using Context)
extends TypeMsg(InvalidMultipleAssignmentTargetID) {
def msg(using Context) =
i"""invalid target of multiple assignment.
|Multiple assignments admit only one level of nesting."""
def explain(using Context) = ""
}

class MultipleAssignmentShapeMismatch(found: Int, required: Int)(using Context)
extends TypeMsg(MultipleAssignmentShapeMismatchID) {
def msg(using Context) =
i"""Source and target of multiple assignment have different sizes.
|Source: $found
|Target: $required"""
def explain(using Context) = ""
}
27 changes: 27 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/Dynamic.scala
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,33 @@ trait Dynamic {
}
}

/** Returns the partial application of a dynamic assignment translating selection that does not
* typecheck according to the normal rules into a `updateDynamic`.
*
* For example: `foo.bar = baz ~~> foo.updateDynamic(bar)(baz)`
*
* @param lhs The target of the assignment.
*/
def formPartialDynamicAssignment(
lhs: untpd.Tree
)(using Context): PartialAssignment[SimpleLValue] =
lhs match
case s @ Select(q, n) if !isDynamicMethod(n) =>
formPartialDynamicAssignment(q, n, s.span, Nil)
case TypeApply(s @ Select(q, n), targs) if !isDynamicMethod(n) =>
formPartialDynamicAssignment(q, n, s.span, Nil)
case _ =>
val e = errorTree(lhs, ReassignmentToVal(lhs.symbol.name))
PartialAssignment(SimpleLValue(e)) { (l, _) => l.expression }

def formPartialDynamicAssignment(
q: untpd.Tree, n: Name, s: Span, targs: List[untpd.Tree]
)(using Context): PartialAssignment[SimpleLValue] =
val v = typed(coreDynamic(q, nme.updateDynamic, n, s, targs))
PartialAssignment(SimpleLValue(v)) { (l, r) =>
untpd.Apply(untpd.TypedSplice(l.expression), List(r))
}

private def coreDynamic(qual: untpd.Tree, dynName: Name, name: Name, selSpan: Span, targs: List[untpd.Tree])(using Context): untpd.Apply = {
val select = untpd.Select(qual, dynName).withSpan(selSpan)
val selectWithTypes =
Expand Down
156 changes: 156 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/PartialAssignment.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
package dotty.tools
package dotc
package typer

import dotty.tools.dotc.ast.Trees.ApplyKind
import dotty.tools.dotc.ast.tpd
import dotty.tools.dotc.ast.untpd
import dotty.tools.dotc.ast.TreeInfo
import dotty.tools.dotc.core.Contexts.Context
import dotty.tools.dotc.core.Names.Name
import dotty.tools.dotc.core.NameKinds.TempResultName

import core.Symbols.defn

/** A function computing the assignment of a lvalue.
*
* @param lhs The target of the assignment.
* @param perform: A closure that accepts `lhs` and an untyped tree `rhs`, and returns a tree
* representing the assignment of `rhs` to `lhs`.
*/
private[typer] final class PartialAssignment[+T <: LValue](val lhs: T)(
perform: (T, untpd.Tree) => untpd.Tree
):

/** Returns a tree computing the assignment of `rhs` to `lhs`. */
def apply(rhs: untpd.Tree): untpd.Tree =
perform(lhs, rhs)

end PartialAssignment

/** The expression of a pure value or a synthetic val definition binding a value whose evaluation
* must be hoisted.
*
* Use this type to represent a part of a lvalue that must be evaluated before the lvalue gets
* used for updating a value.
*/
private[typer] final class PossiblyHoistedValue private (representation: tpd.Tree):
Copy link
Member

Choose a reason for hiding this comment

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

Consider putting all those helper classes inside an object PartialAssignment. It's unusual to have top-level classes and objects that don't match the enclosing file name.

Copy link
Member

Choose a reason for hiding this comment

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

Rather than having a single class with a basically-untyped polymorphic argument, consider defining this as a sealed abstract class with two subclasses HoistedValue and DirectValue (naming TBD).


/** Returns a tree representing the value of `self`. */
def value(using Context): tpd.Tree =
definition match
case Some(d) => tpd.Ident(d.namedType).withSpan(representation.span)
Copy link
Member

Choose a reason for hiding this comment

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

Is this equivalent to

Suggested change
case Some(d) => tpd.Ident(d.namedType).withSpan(representation.span)
case Some(d) => ref(d).withSpan(representation.span)

?

case _ => representation

/** Returns the synthetic val defining `self` if it is hoisted. */
def definition: Option[tpd.ValDef] =
representation match
case d: tpd.ValDef => Some(d)
case _ => None

/** Returns a tree representing the value of `self` along with its hoisted definition, if any. */
def valueAndDefinition(using Context): (tpd.Tree, Option[tpd.ValDef]) =
definition match
Comment on lines +52 to +53
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason not to implement that method as

Suggested change
def valueAndDefinition(using Context): (tpd.Tree, Option[tpd.ValDef]) =
definition match
def valueAndDefinition(using Context): (tpd.Tree, Option[tpd.ValDef]) =
(value, definition)

?

case Some(d) => (tpd.Ident(d.namedType).withSpan(representation.span), Some(d))
case _ => (representation, None)

object PossiblyHoistedValue:

/** Creates a value representing the `e`'s evaluation. */
def apply(e: tpd.Tree, isSingleAssignment: Boolean)(using Context): PossiblyHoistedValue =
if isSingleAssignment || (tpd.exprPurity(e) >= TreeInfo.Pure) then
new PossiblyHoistedValue(e)
else
new PossiblyHoistedValue(tpd.SyntheticValDef(TempResultName.fresh(), e).withSpan(e.span))

/** The left-hand side of an assignment. */
private[typer] sealed abstract class LValue:

/** Returns the local `val` definitions composing this lvalue. */
def locals: List[tpd.ValDef]

/** Returns a tree computing the assignment of `rhs` to this lvalue. */
def formAssignment(rhs: untpd.Tree)(using Context): untpd.Tree
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def formAssignment(rhs: untpd.Tree)(using Context): untpd.Tree
def genAssignment(rhs: untpd.Tree)(using Context): untpd.Tree

might better follow typical naming conventions of the codebase?


end LValue

/** A simple expression, typically valid on left-hand side of an `Assign` tree.
*
* Use this class to represent an assignment that translates to an `Assign` tree or to wrap an
* error whose diagnostic can be delayed until the right-hand side is known.
*
* @param expression The expression of the lvalue.
*/
private[typer] final case class SimpleLValue(expression: tpd.Tree) extends LValue:

def locals: List[tpd.ValDef] =
List()

def formAssignment(rhs: untpd.Tree)(using Context): untpd.Tree =
val s = untpd.Assign(untpd.TypedSplice(expression), rhs)
untpd.TypedSplice(s.withType(defn.UnitType))
Comment on lines +89 to +91
Copy link
Member

Choose a reason for hiding this comment

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

This seems suspicious. IIRC, a TypedSplice assumes that its inside is deeply typed, since TypeAssigner/Typer won't dive into it. But here it looks like you smuggle an Assign as typed, whose rhs is untyped.

Why not directly returning s? It is a valid untpd.Assign node.


end SimpleLValue

/** A lvalue represeted by the partial application a function.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/** A lvalue represeted by the partial application a function.
/** A lvalue represented by the partial application a function.

*
* @param function The partially applied function.
* @param arguments The arguments of the partial application.
*/
private[typer] final case class ApplyLValue(
function: ApplyLValue.Callee,
arguments: List[PossiblyHoistedValue]
) extends LValue:

val locals: List[tpd.ValDef] =
function.locals ++ (arguments.flatMap { (v) => v.definition })
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
function.locals ++ (arguments.flatMap { (v) => v.definition })
function.locals ++ arguments.flatMap(_.definition)

?


def formAssignment(rhs: untpd.Tree)(using Context): untpd.Tree =
val s = function.expanded
val t = arguments.map((a) => untpd.TypedSplice(a.value)) :+ rhs
untpd.Apply(s, t)

object ApplyLValue:

/** The callee of a lvalue represented by a partial application. */
sealed abstract class Callee:

/** Returns the tree representing this callee. */
def expanded(using Context): untpd.Tree

/** Returns the local `val` definitions composing this lvalue. */
def locals: List[tpd.ValDef]

object Callee:

/** Creates an instance from a function represented as a typed tree. */
def apply(
receiver: tpd.Tree, isSingleAssignment: Boolean
)(using Context): Typed =
Typed(PossiblyHoistedValue(receiver, isSingleAssignment), None)

/** Creates an instance denoting a selection on a receiver represented as a typed tree. */
def apply(
receiver: tpd.Tree, member: Name, isSingleAssignment: Boolean
)(using Context): Typed =
Typed(PossiblyHoistedValue(receiver, isSingleAssignment), Some(member))

/** A function representing a lvalue. */
final case class Typed(receiver: PossiblyHoistedValue, member: Option[Name]) extends Callee:

def expanded(using Context): untpd.Tree =
val s = untpd.TypedSplice(receiver.value)
member.map((m) => untpd.Select(s, m)).getOrElse(s)

def locals: List[tpd.ValDef] =
receiver.definition.toList

/** The untyped expression of a function representing a lvalue along with its captures. */
final case class Untyped(value: untpd.Tree, locals: List[tpd.ValDef]) extends Callee:

def expanded(using Context): untpd.Tree =
value

end Callee

end ApplyLValue
Loading
Loading