Skip to content

Commit

Permalink
Introduced Def::external_ flag
Browse files Browse the repository at this point in the history
Solely relying on the name introduces subtle bugs when some Def's
name coincidentally has the same name as an external. This flag
distinguishes externals from non-externals with the same name and also
makes Def::is_external checks faster.
  • Loading branch information
leissa committed Mar 8, 2023
1 parent c6c4e45 commit 2997a1d
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 11 deletions.
3 changes: 1 addition & 2 deletions lit/direct/ds2cps_ax_cps2ds_dependent2.thorin
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,4 @@
return (mem, c)
};

// for some reasone eta-red doesn't always work; so don't check this: return{{.*}}47
// CHECK-DAG: 47:(.Idx 4294967296)
// CHECK-DAG: return{{.*}}47
3 changes: 2 additions & 1 deletion thorin/def.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ Def::Def(World* w, node_t node, const Def* type, Defs ops, flags_t flags)
, flags_(flags)
, node_(unsigned(node))
, nom_(false)
, external_(false)
, dep_(unsigned(node == Node::Axiom ? Dep::Axiom
: node == Node::Infer ? Dep::Infer
: node == Node::Proxy ? Dep::Proxy
Expand Down Expand Up @@ -54,6 +55,7 @@ Def::Def(node_t node, const Def* type, size_t num_ops, flags_t flags)
: flags_(flags)
, node_(node)
, nom_(true)
, external_(false)
, dep_(Dep::Nom | (node == Node::Infer ? Dep::Infer : Dep::None))
, num_ops_(num_ops)
, type_(type) {
Expand Down Expand Up @@ -370,7 +372,6 @@ bool Def::is_set() const {

void Def::make_external() { return world().make_external(this); }
void Def::make_internal() { return world().make_internal(this); }
bool Def::is_external() const { return world().is_external(this); }

std::string Def::unique_name() const { return *sym() + "_"s + std::to_string(gid()); }

Expand Down
11 changes: 6 additions & 5 deletions thorin/def.h
Original file line number Diff line number Diff line change
Expand Up @@ -372,8 +372,8 @@ class Def : public RuntimeCast<Def> {

/// @name externals
///@{
bool is_external() const;
bool is_internal() const { return !is_external(); } ///< *Not* Def::is_external.
bool is_external() const { return external_; }
bool is_internal() const { return !is_external(); } ///< **Not** Def::is_external.
void make_external();
void make_internal();
///@}
Expand Down Expand Up @@ -493,9 +493,10 @@ class Def : public RuntimeCast<Def> {

flags_t flags_;
uint8_t node_;
unsigned nom_ : 1;
unsigned dep_ : 5;
unsigned pading_ : 2;
unsigned nom_ : 1;
unsigned external_ : 1;
unsigned dep_ : 5;
unsigned pading_ : 1;
u8 curry_;
u8 trip_;
hash_t hash_;
Expand Down
10 changes: 7 additions & 3 deletions thorin/world.h
Original file line number Diff line number Diff line change
Expand Up @@ -151,12 +151,16 @@ class World {
const auto& externals() const { return move_.externals; }
bool empty() { return move_.externals.empty(); }
void make_external(Def* def) {
def->external_ = true;
assert(def->sym());
move_.externals.emplace(def->sym(), def); // TODO enable assert again
// auto [i, ins] = move_.externals.emplace(name, def);
// assert((ins || (def == i->second)) && "two different externals registered with the same name");
}
void make_internal(Def* def) { move_.externals.erase(def->sym()); }
void make_internal(Def* def) {
def->external_ = false;
move_.externals.erase(def->sym());
}
bool is_external(Ref def) { return move_.externals.contains(def->sym()); }
Def* lookup(Sym name) {
auto i = move_.externals.find(name);
Expand Down Expand Up @@ -463,7 +467,7 @@ class World {
if (auto loc = emit_loc()) def->set(loc);
assert(!def->isa_nom());
#if THORIN_ENABLE_CHECKS
if (flags().trace_gids) outln("{}: {}", def->node_name(), def->gid());
if (flags().trace_gids) outln("{}: {} - {}", def->node_name(), def->gid(), def->flags());
if (flags().reeval_breakpoints && breakpoints().contains(def->gid())) thorin::breakpoint();
#endif
if (is_frozen()) {
Expand All @@ -490,7 +494,7 @@ class World {
auto def = arena_.allocate<T>(num_ops, std::forward<Args&&>(args)...);
if (auto loc = emit_loc()) def->set(loc);
#if THORIN_ENABLE_CHECKS
if (flags().trace_gids) outln("{}: {}", def->node_name(), def->gid());
if (flags().trace_gids) outln("{}: {} - {}", def->node_name(), def->gid(), def->flags());
if (breakpoints().contains(def->gid())) thorin::breakpoint();
#endif
auto [_, ins] = move_.defs.emplace(def);
Expand Down

0 comments on commit 2997a1d

Please sign in to comment.