Skip to content

Commit

Permalink
Support arbitrary expression as argument to interval(...).
Browse files Browse the repository at this point in the history
So far we used constructor expressions like `interval(...)` to create
*constants* of the given type, and for that we required that the
argument was a single atomic value. The result was that these
constructor expressions were really more like literals for the
corresponding types then "normal" expressions. While that's useful
internally, it's confusing for the user. This commits is beginning to
change that, using `interval` as the first instance that now supports
arbitrary expressions as its argument. In follow-up commits, we'll
port other constructor expressions over to that new syntax.

To change this without loosing a way to create actual constants (which
we need at some places), we implement this in two stages: the parser
initially turns such `interval(...)` expressions into `cast` nodes
(i.e., `cast<interval>(...)`). Such casts are already supported to
create an interval from an integer, so that just works, but it means
we don't have constants anymore. To get them back where we can, we
extend the normalizer to look at all such casts: if they are just
receiving a constant value as their argument, then it substitutes them
with a plain ``ctor::Interval`` node-—-which is what we used to have
there.

That check for "if they just receive a constant value" isn't quite
trivial in general. In our case, where we just want to go back to what
we had, it's not that difficult, but more generally it would be nice
if we had a constant folder that could compute results even for more
complex, but constant, expressions. As that's a piece that our AST
infrastructure is still missing, this commit starts going that route
by adding a new "constant folder" visitor. Right now, it's covers only
the simple cases we need here, but it's something we can extend going
forward.

Addresses #1123.

TODO: Not quite sure how to test the substitution as it's only visible
inside the AST.
  • Loading branch information
rsmmr committed Dec 5, 2022
1 parent 25d9595 commit bb7afe2
Show file tree
Hide file tree
Showing 8 changed files with 133 additions and 20 deletions.
1 change: 1 addition & 0 deletions hilti/toolchain/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ set(SOURCES
src/compiler/plugin.cc
src/compiler/unit.cc
src/compiler/visitors/coercer.cc
src/compiler/visitors/constant-folder.cc
src/compiler/visitors/normalizer.cc
src/compiler/visitors/printer.cc
src/compiler/visitors/renderer.cc
Expand Down
27 changes: 27 additions & 0 deletions hilti/toolchain/include/compiler/detail/visitors.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,33 @@ std::string renderOperatorInstance(const expression::ResolvedOperator& o);
void renderNode(const Node& n, std::ostream& out, bool include_scopes = false);
void renderNode(const Node& n, logging::DebugStream stream, bool include_scopes = false);

/**
* Folds an expression into a constant value if that's possible. Note that the
* current implementation is very, very basic, and covers just a few cases. If
* the function returns an error, that does not necessarily mean that the
* expression is not represeneting a constant value, but only that we aren't
* able to compute it.
*/
Result<Ctor> foldConstant(const Expression& expr);

/**
* Folds an expression intoa constant value of a specific type, if that's
* possible. This behaves like the non-templated version of `foldConstant()`
* but adds a check that the resulting ``Ctor`` is of the exepcted type. If
* not, it will fail.
*/
template<typename Ctor>
Result<Ctor> foldConstant(const Expression& expr) {
auto ctor = foldConstant(expr);
if ( ! ctor )
return ctor.error();

if ( auto ctor_ = ctor->tryAs<Ctor>() )
return *ctor_;
else
return result::Error("unexpected type");
}

namespace ast {
/** Implements the corresponding functionality for the default HILTI compiler plugin. */
void buildScopes(const std::shared_ptr<hilti::Context>& ctx, Node* root, Unit* unit);
Expand Down
12 changes: 1 addition & 11 deletions hilti/toolchain/src/compiler/parser/parser.yy
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,6 @@ static uint64_t check_int64_range(uint64_t x, bool positive, const hilti::Meta&
%type <std::pair<std::vector<hilti::Declaration>, std::vector<hilti::Statement>>> global_scope_items

%type <double> const_real
%type <int64_t> const_sint
%type <uint64_t> const_uint

%%
Expand Down Expand Up @@ -786,6 +785,7 @@ expr_e : BEGIN_ '(' expr ')' { $$ = hilti::expression::Unres

expr_f : ctor { $$ = hilti::expression::Ctor(std::move($1), __loc__); }
| PORT '(' expr ',' expr ')' { $$ = hilti::builder::port(std::move($3), std::move($5), __loc__); }
| INTERVAL '(' expr ')' { $$ = hilti::expression::UnresolvedOperator(hilti::operator_::Kind::Cast, { std::move($3), hilti::expression::Type_(hilti::type::Interval())}, __loc__); }
| '-' expr_g { $$ = hilti::expression::UnresolvedOperator(hilti::operator_::Kind::SignNeg, {std::move($2)}, __loc__); }
| '[' expr FOR local_id IN expr ']'
{ $$ = hilti::expression::ListComprehension(std::move($6), std::move($2), std::move($4), {}, __loc__); }
Expand Down Expand Up @@ -814,12 +814,6 @@ ctor : CBOOL { $$ = hilti::ctor::Bool($1, __
| CADDRESS { $$ = hilti::ctor::Address(hilti::ctor::Address::Value($1), __loc__); }
| CADDRESS '/' CUINTEGER { $$ = hilti::ctor::Network(hilti::ctor::Network::Value($1, $3), __loc__); }
| CPORT { $$ = hilti::ctor::Port(hilti::ctor::Port::Value($1), __loc__); }
| INTERVAL '(' const_real ')' { try { $$ = hilti::ctor::Interval(hilti::ctor::Interval::Value($3, hilti::rt::Interval::SecondTag()), __loc__); }
catch ( hilti::rt::OutOfRange& e ) { $$ = hilti::ctor::Interval(hilti::ctor::Interval::Value()); error(@$, e.what()); }
}
| INTERVAL '(' const_sint ')' { try { $$ = hilti::ctor::Interval(hilti::ctor::Interval::Value($3, hilti::rt::Interval::SecondTag()), __loc__); }
catch ( hilti::rt::OutOfRange& e ) { $$ = hilti::ctor::Interval(hilti::ctor::Interval::Value()); error(@$, e.what()); }
}
| TIME '(' const_real ')' { try { $$ = hilti::ctor::Time(hilti::ctor::Time::Value($3, hilti::rt::Time::SecondTag()), __loc__); }
catch ( hilti::rt::OutOfRange& e ) { $$ = hilti::ctor::Time(hilti::ctor::Time::Value()); error(@$, e.what()); }
}
Expand Down Expand Up @@ -852,10 +846,6 @@ const_real : CUREAL { $$ = $1; }
| '+' CUREAL { $$ = $2; }
| '-' CUREAL { $$ = -$2; }

const_sint : CUINTEGER { $$ = check_int64_range($1, true, __loc__); }
| '+' CUINTEGER { $$ = check_int64_range($2, true, __loc__); }
| '-' CUINTEGER { $$ = -check_int64_range($2, false, __loc__); }

const_uint : CUINTEGER { $$ = $1; }
| '+' CUINTEGER { $$ = $2; }

Expand Down
48 changes: 48 additions & 0 deletions hilti/toolchain/src/compiler/visitors/constant-folder.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright (c) 2020-2021 by the Zeek Project. See LICENSE for details.

#include <hilti/ast/ctors/integer.h>
#include <hilti/ast/detail/visitor.h>
#include <hilti/ast/expression.h>
#include <hilti/ast/operators/signed-integer.h>
#include <hilti/ast/operators/real.h>
#include <hilti/ast/types/integer.h>
#include <hilti/base/logger.h>
#include <hilti/base/util.h>
#include <hilti/ast/builder/expression.h>
#include <hilti/compiler/detail/visitors.h>

using namespace hilti;

namespace {

// For now, this is only a very basic constant folder that only covers cases we
// need to turn type constructor expressions coming with a single argument into
// ctor expressions.
struct VisitorConstantFolder : public visitor::PreOrder<Ctor, VisitorConstantFolder> {
result_t operator()(const expression::Ctor& n, position_t p) { return n.ctor(); }

result_t operator()(const operator_::signed_integer::SignNeg& n, position_t p) {
if ( auto op = detail::foldConstant<ctor::SignedInteger>(n.op1()) )
return ctor::SignedInteger(- op->value(), op->width(), op->meta());
else
return {};
}

result_t operator()(const operator_::real::SignNeg& n, position_t p) {
if ( auto op = detail::foldConstant<ctor::Real>(n.op1()) )
return ctor::Real(- op->value(), op->meta());
else
return {};
}
};

} // anonymous namespace

Result<Ctor> detail::foldConstant(const Expression& expr) {
auto v = VisitorConstantFolder();

if ( auto ctor = v.dispatch(expr) )
return *ctor;
else
return result::Error("not a foldable constant expression");
}
49 changes: 48 additions & 1 deletion hilti/toolchain/src/compiler/visitors/normalizer.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Copyright (c) 2020-2021 by the Zeek Project. See LICENSE for details.

#include <hilti/ast/ctors/integer.h>
#include <hilti/ast/ctors/interval.h>
#include <hilti/ast/declarations/global-variable.h>
#include <hilti/ast/declarations/imported-module.h>
#include <hilti/ast/detail/operator-registry.h>
Expand All @@ -20,12 +22,18 @@ namespace {
struct VisitorNormalizer : public visitor::PreOrder<void, VisitorNormalizer> {
bool modified = false;

// Log debug message recording resolving a epxxression.
// Log debug message recording resolving a expression.
void logChange(const Node& old, const Expression& nexpr) {
HILTI_DEBUG(logging::debug::Normalizer,
util::fmt("[%s] %s -> expression %s (%s)", old.typename_(), old, nexpr, old.location()));
}

// Log debug message recording resolving a ctor.
void logChange(const Node& old, const Ctor& nexpr) {
HILTI_DEBUG(logging::debug::Normalizer,
util::fmt("[%s] %s -> ctor %s (%s)", old.typename_(), old, nexpr, old.location()));
}

// Log debug message recording resolving a statement.
void logChange(const Node& old, const Statement& nstmt) {
HILTI_DEBUG(logging::debug::Normalizer,
Expand Down Expand Up @@ -119,6 +127,45 @@ struct VisitorNormalizer : public visitor::PreOrder<void, VisitorNormalizer> {
}
}

void operator()(const operator_::signed_integer::CastToInterval& cast, position_t p) {
if ( auto ctor = detail::foldConstant<ctor::SignedInteger>(cast.op0()) ) {
try {
auto i = ctor::Interval(ctor::Interval::Value(ctor->value(), hilti::rt::Interval::SecondTag()));
logChange(p.node, i);
p.node = expression::Ctor(std::move(i));
modified = true;
} catch ( hilti::rt::OutOfRange& e ) {
p.node.addError(e.what());
}
}
}

void operator()(const operator_::unsigned_integer::CastToInterval& cast, position_t p) {
if ( auto ctor = detail::foldConstant<ctor::UnsignedInteger>(cast.op0()) ) {
try {
auto i = ctor::Interval(ctor::Interval::Value(ctor->value(), hilti::rt::Interval::SecondTag()));
logChange(p.node, i);
p.node = expression::Ctor(std::move(i));
modified = true;
} catch ( hilti::rt::OutOfRange& e ) {
p.node.addError(e.what());
}
}
}

void operator()(const operator_::real::CastToInterval& cast, position_t p) {
if ( auto ctor = detail::foldConstant<ctor::Real>(cast.op0()) ) {
try {
auto i = ctor::Interval(hilti::ctor::Interval::Value(ctor->value(), hilti::rt::Interval::SecondTag()));
logChange(p.node, i);
p.node = expression::Ctor(std::move(i));
modified = true;
} catch ( hilti::rt::OutOfRange& e ) {
p.node.addError(e.what());
}
}
}

void operator()(const statement::If& n, position_t p) {
if ( n.init() && ! n.condition() ) {
auto cond = expression::UnresolvedID(n.init()->id());
Expand Down
9 changes: 2 additions & 7 deletions spicy/toolchain/src/compiler/parser/parser.yy
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ namespace spicy { namespace detail { class Parser; } }
%verbose

%glr-parser
%expect 131
%expect 130
%expect-rr 160

%{
Expand Down Expand Up @@ -912,6 +912,7 @@ expr_e : CAST type_param_begin type type_param_end '(' expr ')' { $$ =

expr_f : ctor { $$ = hilti::expression::Ctor(std::move($1), __loc__); }
| PORT '(' expr ',' expr ')' { $$ = hilti::builder::port(std::move($3), std::move($5), __loc__); }
| INTERVAL '(' expr ')' { $$ = hilti::expression::UnresolvedOperator(hilti::operator_::Kind::Cast, { std::move($3), hilti::expression::Type_(hilti::type::Interval())}, __loc__); }
| '-' expr_g { $$ = hilti::expression::UnresolvedOperator(hilti::operator_::Kind::SignNeg, {std::move($2)}, __loc__); }
| '[' expr FOR local_id IN expr ']'
{ $$ = hilti::expression::ListComprehension(std::move($6), std::move($2), std::move($4), {}, __loc__); }
Expand Down Expand Up @@ -945,12 +946,6 @@ ctor : CADDRESS { $$ = hilti::ctor::Address(hil
| '+' CUINTEGER { $$ = hilti::ctor::SignedInteger(check_int64_range($2, true, __loc__), 64, __loc__); }
| '-' CUINTEGER { $$ = hilti::ctor::SignedInteger(-check_int64_range($2, false, __loc__), 64, __loc__); }
| OPTIONAL '(' expr ')' { $$ = hilti::ctor::Optional(std::move($3), __loc__); }
| INTERVAL '(' const_real ')' { try { $$ = hilti::ctor::Interval(hilti::ctor::Interval::Value($3, hilti::rt::Interval::SecondTag()), __loc__); }
catch ( hilti::rt::OutOfRange& e ) { $$ = hilti::ctor::Interval(hilti::ctor::Interval::Value()); error(@$, e.what()); }
}
| INTERVAL '(' const_sint ')' { try { $$ = hilti::ctor::Interval(hilti::ctor::Interval::Value($3, hilti::rt::Interval::SecondTag()), __loc__); }
catch ( hilti::rt::OutOfRange& e ) { $$ = hilti::ctor::Interval(hilti::ctor::Interval::Value()); error(@$, e.what()); }
}
| INTERVAL_NS '(' const_sint ')' { $$ = hilti::ctor::Interval(hilti::ctor::Interval::Value($3, hilti::rt::Interval::NanosecondTag()), __loc__); }
| TIME '(' const_real ')' { try { $$ = hilti::ctor::Time(hilti::ctor::Time::Value($3, hilti::rt::Time::SecondTag()), __loc__); }
catch ( hilti::rt::OutOfRange& e ) { $$ = hilti::ctor::Time(hilti::ctor::Time::Value()); error(@$, e.what()); }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
[error] <...>/ctor-out-of-range.spicy:7:12: value cannot be represented as an interval
[error] <...>/ctor-out-of-range.spicy:8:12: value cannot be represented as an interval
[error] spicyc: parse error
[error] spicyc: aborting after errors
5 changes: 5 additions & 0 deletions tests/hilti/types/interval/ops.hlt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import hilti;

global i1 = interval(2.5);
global i2 = interval(1.5);
global i3 = interval(10 + 15);
global i4 = interval(10.0 + 15.0);

hilti::print(i1);
hilti::print(i2);
Expand All @@ -30,4 +32,7 @@ assert i2 <= i2;
assert cast<interval>(5) == interval(5.0);
assert cast<interval>(5.0) == interval(5.0);

assert i3 == interval(25.0);
assert i3 == i4;

}

0 comments on commit bb7afe2

Please sign in to comment.