Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/topic/robin/gh-1143-optional-coe…
Browse files Browse the repository at this point in the history
…rce'

* origin/topic/robin/gh-1143-optional-coerce:
  Add coercion on assignment for optionals that only differ in constness of their inner types.
  • Loading branch information
rsmmr committed Aug 18, 2022
2 parents 2e8fdb6 + 14478b2 commit 8d65789
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 @@ -343,6 +343,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 @@ -292,10 +292,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 8d65789

Please sign in to comment.