Skip to content

Commit 42c2139

Browse files
amomchilovamomchilov
committed
Migrate op assigns to return ExpressionPtr 
1 parent 615b0e4 commit 42c2139

File tree

2 files changed

+28
-39
lines changed

2 files changed

+28
-39
lines changed

‎parser/prism/Translator.cc‎

Lines changed: 21 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -594,8 +594,8 @@ ast::Send *asTLet(ExpressionPtr &arg) {
594594
// TODO: narrow the type back after direct desugaring is complete. https://github.com/Shopify/sorbet/issues/671
595595
// The location is the location of the whole Prism assignment node.
596596
template <typename PrismAssignmentNode, typename SorbetAssignmentNode, typename SorbetLHSNode>
597-
unique_ptr<ExprOnly> Translator::translateAnyOpAssignment(PrismAssignmentNode *node, core::LocOffsets location,
598-
ast::ExpressionPtr lhs) {
597+
ast::ExpressionPtr Translator::translateAnyOpAssignment(PrismAssignmentNode *node, core::LocOffsets location,
598+
ast::ExpressionPtr lhs) {
599599
auto rhs = desugar(node->value);
600600

601601
if constexpr (is_same_v<SorbetAssignmentNode, parser::AndAsgn>) {
@@ -613,7 +613,7 @@ unique_ptr<ExprOnly> Translator::translateAnyOpAssignment(PrismAssignmentNode *n
613613

614614
// The location is the location of the whole Prism assignment node.
615615
template <typename PrismAssignmentNode, typename SorbetAssignmentNode>
616-
unique_ptr<ExprOnly> Translator::translateIndexAssignment(pm_node_t *untypedNode, core::LocOffsets location) {
616+
ast::ExpressionPtr Translator::translateIndexAssignment(pm_node_t *untypedNode, core::LocOffsets location) {
617617
auto node = down_cast<PrismAssignmentNode>(untypedNode);
618618

619619
// The LHS location includes the receiver and the `[]`, but not the `=` or rhs.
@@ -637,8 +637,8 @@ unique_ptr<ExprOnly> Translator::translateIndexAssignment(pm_node_t *untypedNode
637637

638638
// The location is the location of the whole Prism assignment node.
639639
template <typename SorbetAssignmentNode>
640-
unique_ptr<ExprOnly> Translator::translateAndOrAssignment(core::LocOffsets location, ast::ExpressionPtr lhs,
641-
ast::ExpressionPtr rhs) {
640+
ast::ExpressionPtr Translator::translateAndOrAssignment(core::LocOffsets location, ast::ExpressionPtr lhs,
641+
ast::ExpressionPtr rhs) {
642642
const auto isOrAsgn = is_same_v<SorbetAssignmentNode, parser::OrAsgn>;
643643
const auto isAndAsgn = is_same_v<SorbetAssignmentNode, parser::AndAsgn>;
644644
static_assert(isOrAsgn || isAndAsgn);
@@ -648,8 +648,7 @@ unique_ptr<ExprOnly> Translator::translateAndOrAssignment(core::LocOffsets locat
648648
auto locZeroLen = location.copyWithZeroLength();
649649

650650
// Desugar `x &&= y` to `<Magic>.&&=(x, y)` (likewise for `||=`)
651-
auto magicSend = MK::Send2(location, MK::Magic(locZeroLen), magicName, locZeroLen, move(lhs), move(rhs));
652-
return expr_only(move(magicSend));
651+
return MK::Send2(location, MK::Magic(locZeroLen), magicName, locZeroLen, move(lhs), move(rhs));
653652
}
654653

655654
if (auto s = ast::cast_tree<ast::Send>(lhs)) {
@@ -672,8 +671,7 @@ unique_ptr<ExprOnly> Translator::translateAndOrAssignment(core::LocOffsets locat
672671
// OrAsgn: if (lhs) { lhs } else { lhs = rhs }
673672
if_ = MK::If(sendLoc, MK::Local(sendLoc, tempResult), move(elsep), move(body));
674673
}
675-
auto wrapped = MK::InsSeq(location, move(stats), move(if_));
676-
return expr_only(move(wrapped));
674+
return MK::InsSeq(location, move(stats), move(if_));
677675
}
678676

679677
if (isa_reference(lhs)) {
@@ -717,24 +715,20 @@ unique_ptr<ExprOnly> Translator::translateAndOrAssignment(core::LocOffsets locat
717715
assignExpr = MK::Assign(location, MK::cpRef(lhs), move(rhs));
718716
}
719717

720-
ExpressionPtr if_;
721718
if constexpr (isAndAsgn) {
722719
// AndAsgn: if (lhs) { lhs = rhs } else { lhs }
723-
if_ = MK::If(location, move(cond), move(assignExpr), move(lhsCopy));
720+
return MK::If(location, move(cond), move(assignExpr), move(lhsCopy));
724721
} else {
725722
// OrAsgn: if (lhs) { lhs } else { lhs = rhs }
726-
if_ = MK::If(location, move(cond), move(lhsCopy), move(assignExpr));
723+
return MK::If(location, move(cond), move(lhsCopy), move(assignExpr));
727724
}
728-
729-
return expr_only(move(if_));
730725
}
731726

732727
if (ast::isa_tree<ast::UnresolvedConstantLit>(lhs)) {
733728
if (auto e = ctx.beginIndexerError(location, core::errors::Desugar::NoConstantReassignment)) {
734729
e.setHeader("Constant reassignment is not supported");
735730
}
736-
ExpressionPtr res = MK::EmptyTree();
737-
return expr_only(move(res));
731+
return MK::EmptyTree();
738732
}
739733

740734
if (ast::isa_tree<ast::InsSeq>(lhs)) {
@@ -759,8 +753,7 @@ unique_ptr<ExprOnly> Translator::translateAndOrAssignment(core::LocOffsets locat
759753
numPosAssgnArgs, move(assgnArgs), s->flags);
760754
auto elsep = MK::Local(sendLoc, tempResult);
761755
auto iff = MK::If(sendLoc, MK::Local(sendLoc, tempResult), move(body), move(elsep));
762-
auto wrapped = MK::InsSeq(location, move(stats), move(iff));
763-
return expr_only(move(wrapped));
756+
return MK::InsSeq(location, move(stats), move(iff));
764757
}
765758

766759
Exception::raise("the LHS has been desugared to something we haven't expected: {}", lhs.toString(ctx));
@@ -803,17 +796,16 @@ Translator::OpAsgnScaffolding Translator::copyArgsForOpAsgn(ast::Send *s) {
803796

804797
// The location is the location of the whole Prism assignment node.
805798
template <typename SorbetAssignmentNode, typename PrismAssignmentNode>
806-
unique_ptr<ExprOnly> Translator::translateOpAssignment(PrismAssignmentNode *node, core::LocOffsets location,
807-
ast::ExpressionPtr lhs, ast::ExpressionPtr rhs) {
799+
ast::ExpressionPtr Translator::translateOpAssignment(PrismAssignmentNode *node, core::LocOffsets location,
800+
ast::ExpressionPtr lhs, ast::ExpressionPtr rhs) {
808801
// `OpAsgn` assign needs more information about the specific operator here, so it gets special handling here.
809802
auto opLoc = translateLoc(node->binary_operator_loc);
810803
auto op = translateConstantName(node->binary_operator);
811804

812805
if (preserveConcreteSyntax) {
813806
auto magicName = core::Names::opAsgn();
814807
auto locZeroLen = location.copyWithZeroLength();
815-
auto magicSend = MK::Send2(location, MK::Magic(locZeroLen), magicName, locZeroLen, move(lhs), move(rhs));
816-
return expr_only(move(magicSend));
808+
return MK::Send2(location, MK::Magic(locZeroLen), magicName, locZeroLen, move(lhs), move(rhs));
817809
}
818810

819811
if (ast::isa_tree<ast::Send>(lhs)) {
@@ -836,23 +828,20 @@ unique_ptr<ExprOnly> Translator::translateOpAssignment(PrismAssignmentNode *node
836828
auto res = MK::Send(sendLoc, MK::Local(sendLoc, tempRecv), s->fun.addEq(ctx), sendLoc.copyWithZeroLength(),
837829
numPosAssgnArgs, move(assgnArgs), s->flags);
838830

839-
auto wrapped = MK::InsSeq(location, move(stats), move(res));
840-
return expr_only(move(wrapped));
831+
return MK::InsSeq(location, move(stats), move(res));
841832
}
842833

843834
if (isa_reference(lhs)) {
844835
auto lhsCopy = MK::cpRef(lhs);
845836
auto callOp = MK::Send1(location, move(lhs), op, opLoc, move(rhs));
846-
auto assign = MK::Assign(location, move(lhsCopy), move(callOp));
847-
return expr_only(move(assign));
837+
return MK::Assign(location, move(lhsCopy), move(callOp));
848838
}
849839

850840
if (ast::isa_tree<ast::UnresolvedConstantLit>(lhs)) {
851841
if (auto e = ctx.beginIndexerError(location, core::errors::Desugar::NoConstantReassignment)) {
852842
e.setHeader("Constant reassignment is not supported");
853843
}
854-
ExpressionPtr res = MK::EmptyTree();
855-
return expr_only(move(res));
844+
return MK::EmptyTree();
856845
}
857846

858847
if (auto i = ast::cast_tree<ast::InsSeq>(lhs)) {
@@ -884,7 +873,7 @@ unique_ptr<ExprOnly> Translator::translateOpAssignment(PrismAssignmentNode *node
884873
numPosAssgnArgs, move(assgnArgs), s->flags);
885874
auto wrapped = MK::InsSeq(location, move(stats), move(res));
886875
ifExpr->elsep = move(wrapped);
887-
return expr_only(move(lhs));
876+
return lhs;
888877
}
889878

890879
auto s = fmt::format("the LHS has been desugared to something we haven't expected: {}", lhs.toString(ctx));
@@ -2882,13 +2871,13 @@ unique_ptr<parser::Node> Translator::translate(pm_node_t *node) {
28822871
return expr_only(move(expr));
28832872
}
28842873
case PM_INDEX_AND_WRITE_NODE: { // And-assignment to an index, e.g. `a[i] &&= false`
2885-
return translateIndexAssignment<pm_index_and_write_node, parser::AndAsgn>(node, location);
2874+
return expr_only(translateIndexAssignment<pm_index_and_write_node, parser::AndAsgn>(node, location));
28862875
}
28872876
case PM_INDEX_OPERATOR_WRITE_NODE: { // Compound assignment to an index, e.g. `a[i] += 1`
2888-
return translateIndexAssignment<pm_index_operator_write_node, parser::OpAsgn>(node, location);
2877+
return expr_only(translateIndexAssignment<pm_index_operator_write_node, parser::OpAsgn>(node, location));
28892878
}
28902879
case PM_INDEX_OR_WRITE_NODE: { // Or-assignment to an index, e.g. `a[i] ||= true`
2891-
return translateIndexAssignment<pm_index_or_write_node, parser::OrAsgn>(node, location);
2880+
return expr_only(translateIndexAssignment<pm_index_or_write_node, parser::OrAsgn>(node, location));
28922881
}
28932882
case PM_INDEX_TARGET_NODE: { // Target of an indirect write to an indexed expression
28942883
// ... like `target[0], target[1] = 1, 2`, `rescue => target[0]`, etc.

‎parser/prism/Translator.h‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -140,17 +140,17 @@ class Translator final {
140140
std::unique_ptr<parser::Mlhs> translateMultiTargetLhs(PrismNode *node, core::LocOffsets location);
141141

142142
template <typename PrismAssignmentNode, typename SorbetAssignmentNode, typename SorbetLHSNode>
143-
std::unique_ptr<ExprOnly> translateAnyOpAssignment(PrismAssignmentNode *node, core::LocOffsets location,
144-
ast::ExpressionPtr lhs);
143+
ast::ExpressionPtr translateAnyOpAssignment(PrismAssignmentNode *node, core::LocOffsets location,
144+
ast::ExpressionPtr lhs);
145145

146146
// Translate operator assignment targeting an indexed expression (e.g., `a[0] += 1`).
147147
template <typename PrismAssignmentNode, typename SorbetAssignmentNode>
148-
std::unique_ptr<ExprOnly> translateIndexAssignment(pm_node_t *node, core::LocOffsets location);
148+
ast::ExpressionPtr translateIndexAssignment(pm_node_t *node, core::LocOffsets location);
149149

150150
// Translate AndAsgn/OrAsgn operator assignments (e.g., `x &&= y`, `x ||= y`).
151151
template <typename SorbetAssignmentNode>
152-
std::unique_ptr<ExprOnly> translateAndOrAssignment(core::LocOffsets location, ast::ExpressionPtr lhs,
153-
ast::ExpressionPtr rhs);
152+
ast::ExpressionPtr translateAndOrAssignment(core::LocOffsets location, ast::ExpressionPtr lhs,
153+
ast::ExpressionPtr rhs);
154154

155155
template <typename PrismLhsNode, bool checkForDynamicConstAssign = false>
156156
ast::ExpressionPtr translateConst(pm_node_t *node);
@@ -176,8 +176,8 @@ class Translator final {
176176

177177
// Translate OpAsgn operator assignments
178178
template <typename SorbetAssignmentNode, typename PrismAssignmentNode>
179-
std::unique_ptr<ExprOnly> translateOpAssignment(PrismAssignmentNode *node, core::LocOffsets location,
180-
ast::ExpressionPtr lhs, ast::ExpressionPtr rhs);
179+
ast::ExpressionPtr translateOpAssignment(PrismAssignmentNode *node, core::LocOffsets location,
180+
ast::ExpressionPtr lhs, ast::ExpressionPtr rhs);
181181

182182
// ========================================================================
183183
// Direct Desugaring Functions for Op-Assignment Nodes

0 commit comments

Comments
 (0)