Skip to content

Commit

Permalink
Fix if-condition with switch parsing.
Browse files Browse the repository at this point in the history
The parser generator was ignoring `if` conditions attached to `switch`
constructs. While we actually had a test for this already, turns out
we had recorded a broken baseline. Plus, we were testing only one
variant of `switch` (expression-based, not look-ahead-based). This
implements and tests both variants now.

Closes #1759.
  • Loading branch information
rsmmr committed Jun 13, 2024
1 parent 5a500a8 commit 75a78da
Show file tree
Hide file tree
Showing 10 changed files with 74 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
#pragma once

#include <memory>
#include <set>
#include <string>
#include <utility>
#include <vector>
Expand All @@ -24,12 +23,16 @@ enum class Default { First, Second, None };
class LookAhead : public Production {
public:
LookAhead(ASTContext* /* ctx */, const std::string& symbol, std::unique_ptr<Production> alt1,
std::unique_ptr<Production> alt2, look_ahead::Default def, const Location& l = location::None)
: Production(symbol, l), _alternatives(std::make_pair(std::move(alt1), std::move(alt2))), _default(def) {}
std::unique_ptr<Production> alt2, look_ahead::Default def, Expression* condition,
const Location& l = location::None)
: Production(symbol, l),
_alternatives(std::make_pair(std::move(alt1), std::move(alt2))),
_default(def),
_condition(condition) {}

LookAhead(ASTContext* ctx, const std::string& symbol, std::unique_ptr<Production> alt1,
std::unique_ptr<Production> alt2, const Location& l = location::None)
: LookAhead(ctx, symbol, std::move(alt1), std::move(alt2), look_ahead::Default::None, l) {}
std::unique_ptr<Production> alt2, Expression* condition, const Location& l = location::None)
: LookAhead(ctx, symbol, std::move(alt1), std::move(alt2), look_ahead::Default::None, condition, l) {}

/** Returns the two alternatives. */
std::pair<Production*, Production*> alternatives() const {
Expand All @@ -39,6 +42,9 @@ class LookAhead : public Production {
/** Returns what's the default alternative. */
const auto& default_() const { return _default; }

/** Returns the boolean condition associated with the production, if any. */
auto condition() const { return _condition; }

bool isAtomic() const final { return false; };
bool isEodOk() const final { return isNullable(); };
bool isLiteral() const final { return false; };
Expand Down Expand Up @@ -70,6 +76,7 @@ class LookAhead : public Production {
std::pair<std::unique_ptr<Production>, std::unique_ptr<Production>> _alternatives;

look_ahead::Default _default;
Expression* _condition = nullptr;

std::pair<production::Set, production::Set> _lahs;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,16 @@ class Switch : public Production {
using Cases = std::vector<std::pair<std::vector<Expression*>, std::unique_ptr<Production>>>;

Switch(ASTContext* /* ctx */, const std::string& symbol, Expression* expr, Cases cases,
std::unique_ptr<Production> default_, AttributeSet* attributes, const Location& l = location::None)
std::unique_ptr<Production> default_, AttributeSet* attributes, Expression* condition,
const Location& l = location::None)
: Production(symbol, l),
_expression(expr),
_cases(std::move(cases)),
_default(std::move(default_)),
_attributes(attributes) {}
_attributes(attributes),
_condition(condition) {}

const auto& condition() const { return _condition; }
const auto& cases() const { return _cases; }
const auto* default_() const { return _default.get(); }
const auto& attributes() const { return _attributes; }
Expand All @@ -54,6 +57,7 @@ class Switch : public Production {
Cases _cases;
std::unique_ptr<Production> _default;
AttributeSet* _attributes = nullptr;
Expression* _condition = nullptr;
};

} // namespace spicy::detail::codegen::production
5 changes: 3 additions & 2 deletions spicy/toolchain/src/compiler/codegen/grammar-builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,8 @@ struct Visitor : public visitor::PreOrder {
}

result = std::make_unique<production::Switch>(context(), switch_sym, n->expression(), std::move(cases),
std::move(default_), n->attributes(), n->meta().location());
std::move(default_), n->attributes(), n->condition(),
n->meta().location());
return;
}

Expand Down Expand Up @@ -263,7 +264,7 @@ struct Visitor : public visitor::PreOrder {

auto lah_sym = fmt("%s_lha_%d", switch_sym, i);
auto lah = std::make_unique<production::LookAhead>(context(), lah_sym, std::move(prev), std::move(prod),
d, c->meta().location());
d, n->condition(), c->meta().location());
prev = std::move(lah);
}

Expand Down
12 changes: 12 additions & 0 deletions spicy/toolchain/src/compiler/codegen/parser-builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1494,6 +1494,9 @@ struct ProductionVisitor : public production::Visitor {
}

void operator()(const production::Switch* p) final {
if ( auto c = p->condition() )
pushBuilder(builder()->addIf(c));

builder()->addCall("hilti::debugIndent", {builder()->stringLiteral("spicy")});

if ( const auto& a = p->attributes()->find("&parse-from") )
Expand Down Expand Up @@ -1545,6 +1548,9 @@ struct ProductionVisitor : public production::Visitor {
popState();

builder()->addCall("hilti::debugDedent", {builder()->stringLiteral("spicy")});

if ( p->condition() )
popBuilder();
}

void operator()(const production::Unit* p) final {
Expand Down Expand Up @@ -1702,6 +1708,9 @@ struct ProductionVisitor : public production::Visitor {
auto parseLookAhead(const production::LookAhead& p) {
assert(state().needs_look_ahead);

if ( auto c = p.condition() )
pushBuilder(builder()->addIf(c));

// If we don't have a look-ahead symbol pending, get one.
auto true_ = builder()->addIf(builder()->not_(state().lahead));
pushBuilder(true_);
Expand Down Expand Up @@ -1761,6 +1770,9 @@ struct ProductionVisitor : public production::Visitor {
pb->parseError("no expected look-ahead token found", p.location());
popBuilder();

if ( p.condition() )
popBuilder();

return std::make_pair(builder_alt1, builder_alt2);
}

Expand Down
2 changes: 1 addition & 1 deletion spicy/toolchain/src/compiler/codegen/productions/while.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ void production::While::preprocessLookAhead(ASTContext* ctx, Grammar* grammar) {

auto l1 = std::make_unique<production::LookAhead>(ctx, symbol() + "_l1",
std::make_unique<production::Epsilon>(ctx, location()),
std::move(unresolved), location());
std::move(unresolved), nullptr, location());
auto l1_ref = std::make_unique<production::Reference>(ctx, l1.get());

auto body_ref = std::make_unique<production::Reference>(ctx, _body.get());
Expand Down
5 changes: 3 additions & 2 deletions spicy/toolchain/tests/grammar.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,11 @@ static auto reference(hilti::ASTContext* ctx, const std::unique_ptr<T>& p) {

static auto lookAhead(hilti::ASTContext* ctx, const std::string& symbol,
std::unique_ptr<spicy::detail::codegen::Production> a1,
std::unique_ptr<spicy::detail::codegen::Production> a2) {
std::unique_ptr<spicy::detail::codegen::Production> a2, hilti::Expression* condition = nullptr) {
return std::make_unique<
spicy::detail::codegen::production::LookAhead>(ctx, symbol, std::move(a1), std::move(a2),
spicy::detail::codegen::production::look_ahead::Default::None);
spicy::detail::codegen::production::look_ahead::Default::None,
condition);
}

static auto epsilon(hilti::ASTContext* ctx) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
[$a=b"1", $b1=b"2345", $b2=(not set)]
[$a=b"2", $b1=(not set), $b2=b"2345"]
Mini::test {
a: 1
b1: 2345
}
Mini::test {
a: 2
}
8 changes: 8 additions & 0 deletions tests/Baseline/spicy.types.unit.switch-if-lahead/output
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
Test::Foo {
x1: Test::X {
a: x1
}
x2: Test::X {}
y: Y
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# @TEST-EXEC: ${SPICYC} %INPUT -j -o %INPUT.hlto
# @TEST-EXEC: echo 1234567890 | spicy-driver %INPUT.hlto >output
# @TEST-EXEC: echo 2234567890 | spicy-driver %INPUT.hlto >>output
# @TEST-EXEC: spicyc %INPUT -j -o %INPUT.hlto
# @TEST-EXEC: echo 1234567890 | spicy-dump %INPUT.hlto >output
# @TEST-EXEC: echo 2234567890 | spicy-dump %INPUT.hlto >>output
# @TEST-EXEC: btest-diff output

module Mini;
Expand All @@ -13,6 +13,4 @@ public type test = unit {
b"1" -> b1: bytes &size=4;
b"2" -> b2: bytes &size=4;
} if ( self.a == b"1" );

on %done { print self; }
};
19 changes: 19 additions & 0 deletions tests/spicy/types/unit/switch-if-lahead.spicy
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# @TEST-EXEC: spicyc %INPUT -j -o %INPUT.hlto
# @TEST-EXEC: echo x1Y | spicy-dump %INPUT.hlto >output
# @TEST-EXEC: btest-diff output

module Test;

type X = unit(cond: bool) {
switch {
-> a: b"x1";
-> b: b"x2";
-> c: b"x3";
} if ( cond ) ;
};

public type Foo = unit {
x1: X(True);
x2: X(False);
y: b"Y";
};

0 comments on commit 75a78da

Please sign in to comment.