Skip to content

API BREAK: reduce allocations during symbol resolution #340

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Smattr merged 1 commit into from
Aug 17, 2026
Merged

API BREAK: reduce allocations during symbol resolution #340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions librumur/include/rumur/Symtab.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace rumur {
class RUMUR_API Symtab {

private:
std::vector<std::unordered_map<std::string, Ptr<Node>>> scope;
std::vector<std::unordered_map<std::string, const Node *>> scope;

public:
void open_scope() { scope.emplace_back(); }
Expand All @@ -30,8 +30,16 @@ class RUMUR_API Symtab {
scope.pop_back();
}

void declare(const std::string &name, const Ptr<Node> &value) {
/// make a new symbol available for lookup
///
/// It is assumed `value` will outlive `*this`, and thus can be retained
/// internally.
///
/// @param name Symbol name
/// @param value Node this name should resolve to
void declare(const std::string &name, const Node *value) {
assert(!scope.empty());
assert(value != nullptr);
if (scope.back().count(name) > 0)
throw Error("symbol \"" + name + "\" was previously declared",
value->loc);
Expand All @@ -43,7 +51,7 @@ class RUMUR_API Symtab {
for (auto it = scope.rbegin(); it != scope.rend(); it++) {
auto it2 = it->find(name);
if (it2 != it->end()) {
if (auto ret = dynamic_cast<const U *>(it2->second.get())) {
if (auto ret = dynamic_cast<const U *>(it2->second)) {
return Ptr<U>(ret->clone());
} else {
break;
Expand Down
64 changes: 46 additions & 18 deletions librumur/src/resolve-symbols.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,32 +18,60 @@
#include <rumur/resolve-symbols.h>
#include <rumur/traverse.h>
#include <rumur/validate.h>
#include <stdexcept>
#include <string>
#include <utility>

using namespace rumur;

/// C++11 polyfill for `std::make_unique`
template <typename T, typename... Args>
static std::unique_ptr<T> make_unique(Args &&...args) {
auto raw = new T(std::forward<Args>(args)...);
try {
return std::unique_ptr<T>{raw};
} catch (std::exception &) {
delete raw;
throw;
}
}

namespace {

class Resolver : public Traversal {

private:
Symtab symtab;

/// node copies we allocated that we are responsible for cleaning up
///
/// We can use `std::unique_ptr` instead of `Ptr` here because we never need
/// to copy or subclass these pointers.
std::vector<std::unique_ptr<Node>> heap;

/// create a new managed node pointer
template <typename T, typename... Args> T *make(Args &&...args) {
std::unique_ptr<T> ptr = make_unique<T>(std::forward<Args>(args)...);
heap.push_back(std::move(ptr));
auto ret = dynamic_cast<T *>(heap.back().get());
assert(ret != nullptr);
return ret;
}

public:
Resolver() {

// Open a global scope
symtab.open_scope();

// Teach the symbol table the built ins
auto td = Ptr<TypeDecl>::make("boolean", Boolean, location());
symtab.declare("boolean", td);
TypeDecl *const boolean = make<TypeDecl>("boolean", Boolean, location());
symtab.declare("boolean", boolean);
mpz_class index = 0;
for (const std::pair<std::string, location> &m : Boolean->members) {
symtab.declare(m.first, Ptr<ConstDecl>::make(
m.first, Ptr<Number>::make(index, location()),
Boolean, location()));
ConstDecl *const member = make<ConstDecl>(
m.first, Ptr<Number>::make(index, location()), Boolean, location());
symtab.declare(m.first, member);
index++;
}
}
Expand All @@ -59,7 +87,7 @@ class Resolver : public Traversal {
symtab.open_scope();
for (auto &a : n.aliases) {
dispatch(*a);
symtab.declare(a->name, a);
symtab.declare(a->name, a.get());
}
for (auto &r : n.rules)
dispatch(*r);
Expand All @@ -70,7 +98,7 @@ class Resolver : public Traversal {
symtab.open_scope();
for (auto &a : n.aliases) {
dispatch(*a);
symtab.declare(a->name, a);
symtab.declare(a->name, a.get());
}
for (auto &s : n.body)
dispatch(*s);
Expand Down Expand Up @@ -122,13 +150,13 @@ class Resolver : public Traversal {
mpz_class index = 0;
size_t id = e->unique_id + 1;
for (const std::pair<std::string, location> &m : n.members) {
auto cd = Ptr<ConstDecl>::make(
ConstDecl *const member = make<ConstDecl>(
m.first, Ptr<Number>::make(index, m.second), e, m.second);
// assign this member a unique id so that referrers can use it if need be
assert(id < e->unique_id_limit &&
"number of enum members exceeds what was expected");
cd->unique_id = id;
symtab.declare(m.first, cd);
member->unique_id = id;
symtab.declare(m.first, member);
index++;
id++;
}
Expand Down Expand Up @@ -186,15 +214,15 @@ class Resolver : public Traversal {
// register the function itself, even though its body has not yet been
// resolved, in order to allow contained function calls to resolve to the
// containing function, supporting recursion
symtab.declare(n.name, Ptr<Function>::make(n));
symtab.declare(n.name, &n);
// only register the function parameters now, to avoid their names shadowing
// anything that needs to be resolved during symbol resolution of another
// parameter or the return type
for (auto &p : n.parameters)
symtab.declare(p->name, p);
symtab.declare(p->name, p.get());
for (auto &d : n.decls) {
dispatch(*d);
symtab.declare(d->name, d);
symtab.declare(d->name, d.get());
}
for (auto &s : n.body)
dispatch(*s);
Expand Down Expand Up @@ -287,9 +315,9 @@ class Resolver : public Traversal {
}

if (auto d = dynamic_cast<Decl *>(c.get()))
symtab.declare(d->name, c);
symtab.declare(d->name, c.get());
if (auto f = dynamic_cast<Function *>(c.get()))
symtab.declare(f->name, c);
symtab.declare(f->name, c.get());
}
}

Expand Down Expand Up @@ -358,7 +386,7 @@ class Resolver : public Traversal {

dispatch(*n.decl);

symtab.declare(n.name, n.decl);
symtab.declare(n.name, n.decl.get());
}

void visit_range(Range &n) final {
Expand Down Expand Up @@ -399,7 +427,7 @@ class Resolver : public Traversal {
dispatch(*n.guard);
for (auto &d : n.decls) {
dispatch(*d);
symtab.declare(d->name, d);
symtab.declare(d->name, d.get());
}
for (auto &s : n.body)
dispatch(*s);
Expand All @@ -414,7 +442,7 @@ class Resolver : public Traversal {
dispatch(q);
for (auto &d : n.decls) {
dispatch(*d);
symtab.declare(d->name, d);
symtab.declare(d->name, d.get());
}
for (auto &s : n.body)
dispatch(*s);
Expand Down
Loading