Skip to content

Commit

Permalink
Add coercion for optionals that only differ in constness of their inn…
Browse files Browse the repository at this point in the history
…er types.

There are some situations where that's fine to coerce, so adding that.
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 committed Aug 10, 2022
1 parent 0a4abac commit e38ae28
Show file tree
Hide file tree
Showing 6 changed files with 59 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
16 changes: 16 additions & 0 deletions hilti/toolchain/src/compiler/coercion.cc
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,22 @@ 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) ) {
if ( style & CoercionStyle::Assignment )
// Assignments copy, so it's safe to turn even into the
// destination without considering constness.
return dst;

if ( type::isConstant(s) && type::isConstant(d) )
// It's safe to turn a non-const value into a const value.
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
9 changes: 8 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,17 @@ 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()) ) {
hilti::render(std::cerr, n.true_().type());
std::cerr << "----\n";
hilti::render(std::cerr, n.false_().type());
std::cerr << "---- " << hilti::type::sameExceptForConstness(n.true_().type(), n.false_().type())
<< std::endl;

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
1 change: 1 addition & 0 deletions tests/Baseline/spicy.types.optional.coercion/output
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
19 changes: 19 additions & 0 deletions tests/spicy/types/optional/coercion.spicy
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# @TEST-EXEC: ${SPICYC} -j %INPUT >output
# @TEST-EXEC: btest-diff 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 e38ae28

Please sign in to comment.