Skip to content

Commit

Permalink
Add coercion on assignment for optionals that only differ in constnes…
Browse files Browse the repository at this point in the history
…s of their inner types.

Also adding a coercion for ternary expressions that coerces the 2dn
expression into the type of the 1st (which we already assume provides
the result type).

Closes #1143.
Closes #1220.
  • Loading branch information
rsmmr authored and bbannier committed Aug 22, 2022
1 parent 1db80a6 commit d1b9bd2
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 1 deletion.
2 changes: 2 additions & 0 deletions hilti/toolchain/include/ast/expressions/ternary.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ class Ternary : public NodeBase, public trait::isExpression {
return condition() == other.condition() && true_() == other.true_() && false_() == other.false_();
}

void setFalse(const Expression& expr) { children()[2] = expr; }

/** Implements `Expression` interface. */
bool isLhs() const { return false; }
/** Implements `Expression` interface. */
Expand Down
10 changes: 10 additions & 0 deletions hilti/toolchain/src/compiler/coercion.cc
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,16 @@ struct VisitorType : public visitor::PreOrder<std::optional<Type>, VisitorType>
}

result_t operator()(const type::Optional& r) {
if ( auto t = dst.tryAs<type::Optional>() ) {
const auto& s = r.dereferencedType();
const auto& d = t->dereferencedType();

if ( type::sameExceptForConstness(s, d) && (style & CoercionStyle::Assignment) )
// Assignments copy, so it's safe to turn into the
// destination without considering constness.
return dst;
}

if ( auto t = dst.tryAs<type::Bool>(); (style & CoercionStyle::ContextualConversion) && t )
return dst;

Expand Down
13 changes: 13 additions & 0 deletions hilti/toolchain/src/compiler/visitors/coercer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,19 @@ struct Visitor : public visitor::PreOrder<void, Visitor> {
}
}

void operator()(const expression::Ternary& n, position_t p) {
if ( ! (type::isResolved(n.true_().type()) && type::isResolved(n.false_().type())) )
return;

// Coerce the second branch to the type of the first. This isn't quite
// ideal, but as good as we can do right now.
if ( auto coerced = coerceExpression(n.false_(), n.true_().type()); coerced && coerced.nexpr ) {
logChange(p.node, *coerced.nexpr, "ternary");
p.node.as<expression::Ternary>().setFalse(*coerced.nexpr);
modified = true;
}
}

void operator()(const operator_::generic::New& n, position_t p) {
auto etype = n.op0().tryAs<expression::Type_>();
if ( ! etype )
Expand Down
3 changes: 2 additions & 1 deletion hilti/toolchain/src/compiler/visitors/validator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -276,10 +276,11 @@ struct VisitorPost : public hilti::visitor::PreOrder<void, VisitorPost>, public
}

void operator()(const expression::Ternary& n, position_t p) {
if ( ! hilti::type::sameExceptForConstness(n.true_().type(), n.false_().type()) )
if ( ! hilti::type::sameExceptForConstness(n.true_().type(), n.false_().type()) ) {
error(fmt("types of alternatives do not match in ternary expression (%s vs. %s)", n.true_().type(),
n.false_().type()),
p);
}
}

void operator()(const expression::UnresolvedID& n, position_t p) {
Expand Down
18 changes: 18 additions & 0 deletions tests/spicy/types/optional/coercion.spicy
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# @TEST-EXEC: ${SPICYC} -j %INPUT >output
#
# @TEST-DOC: Regression test for #1143.

module Foo;

type Bar = struct {
wutang: bytes &optional;
};

type BarTuple = tuple<
wutang: optional<bytes>
>;

public function make_bar_tuple(bar: Bar): BarTuple {
local nope: optional<bytes>;
return tuple(bar?.wutang ? optional(bar.wutang) : nope);
}

0 comments on commit d1b9bd2

Please sign in to comment.