Skip to content

Commit

Permalink
[mycpp/runtime] Remove NewDict() APIs
Browse files Browse the repository at this point in the history
These are redundant with Alloc<Dict<K, V>>.

- We no longer have the rooting problem that led to NewDict<>.
  - Dict objects are rooted when they are nullptr, so it's OK to
    allocate in the constructor.
- mylib.NewDict() is now translated to Alloc<Dict<K, V>> by mycpp.

Fix some unit tests, and also rebuild prebuilt/.
  • Loading branch information
Andy C committed Aug 23, 2023
1 parent d848e64 commit c415837
Show file tree
Hide file tree
Showing 12 changed files with 304 additions and 156 deletions.
4 changes: 2 additions & 2 deletions asdl/gen_cpp_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,8 @@ TEST dicts_test() {
log("m.ss = %p", m->ss);
log("m.ib = %p", m->ib);

m->ss = NewDict<Str*, Str*>();
m->ib = NewDict<int, bool>();
m->ss = Alloc<Dict<Str*, Str*>>();
m->ib = Alloc<Dict<int, bool>>();

m->ss->set(StrFromC("foo"), StrFromC("bar"));

Expand Down
2 changes: 1 addition & 1 deletion cpp/core.cc
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ Str* ReadLine() {
}

Dict<Str*, Str*>* Environ() {
auto d = NewDict<Str*, Str*>();
auto d = Alloc<Dict<Str*, Str*>>();

for (char** env = environ; *env; ++env) {
char* pair = *env;
Expand Down
6 changes: 6 additions & 0 deletions cpp/frontend_flag_spec.cc
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,12 @@ void _CreateActions(Action_c* in, Dict<Str*, args::_Action*>* out) {
}
}

// Convenience function
template <typename K, typename V>
Dict<K, V>* NewDict() {
return Alloc<Dict<K, V>>();
}

// "Inflate" the static C data into a heap-allocated ASDL data structure.
//
// TODO: Make a GLOBAL CACHE? It could be shared between subinterpreters even?
Expand Down
4 changes: 2 additions & 2 deletions frontend/arg_types_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ using runtime_asdl::value;
TEST opaque_test() {
// struct for command -v is HeapTag::Opaque

auto attrs = NewDict<Str*, runtime_asdl::value_t*>();
auto attrs = Alloc<Dict<Str*, runtime_asdl::value_t*>>();
StackRoots _r({&attrs});

auto b = Alloc<value::Bool>(true);
Expand All @@ -28,7 +28,7 @@ TEST opaque_test() {
TEST pointer_test() {
// struct for printf -v is has Str*

auto attrs = NewDict<Str*, runtime_asdl::value_t*>();
auto attrs = Alloc<Dict<Str*, runtime_asdl::value_t*>>();
StackRoots _r({&attrs});

auto s = Alloc<value::Str>(StrFromC("hi %s"));
Expand Down
8 changes: 3 additions & 5 deletions mycpp/cppgen_pass.py
Original file line number Diff line number Diff line change
Expand Up @@ -1060,10 +1060,8 @@ def visit_dict_expr(self, o: 'mypy.nodes.DictExpr') -> T:
key_c_type = GetCType(key_type)
val_c_type = GetCType(val_type)

# Use NewDict<K, V> not Alloc<K, V>
# Could we go back to Alloc<K, V> ?

self.write('NewDict<%s, %s>(' % (key_c_type, val_c_type))
self.write('Alloc<%s>(' % c_type)
#self.write('NewDict<%s, %s>(' % (key_c_type, val_c_type))
if o.items:
keys = [k for k, _ in o.items]
values = [v for _, v in o.items]
Expand Down Expand Up @@ -1329,7 +1327,7 @@ def visit_assignment_stmt(self, o: 'mypy.nodes.AssignmentStmt') -> T:
key_c_type = GetCType(key_type)
val_c_type = GetCType(val_type)

self.write_ind('auto* %s = NewDict<%s, %s>();\n', lval.name,
self.write_ind('auto* %s = Alloc<Dict<%s, %s>>();\n', lval.name,
key_c_type, val_c_type)
return

Expand Down
16 changes: 12 additions & 4 deletions mycpp/gc_dict.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,20 @@ class Dict {
values_(nullptr) {
}

#if 0
Dict(std::initializer_list<K> keys, std::initializer_list<V> values)
: len_(0),
capacity_(0),
entry_(nullptr),
keys_(nullptr),
values_(nullptr) {
assert(keys.size() == values.size());
auto v = values.begin(); // This simulates a "zip" loop
for (auto key : keys) {
// note: calls reserve(), and maybe allocate
this->set(key, *v);
++v;
}
}
#endif

// This relies on the fact that containers of 4-byte ints are reduced by 2
// items, which is greater than (or equal to) the reduction of any other
Expand Down Expand Up @@ -178,7 +183,8 @@ inline bool dict_contains(Dict<K, V>* haystack, K needle) {
return haystack->position_of_key(needle) != -1;
}

// TODO: Remove one of these styles using mycpp code gen
#if 0
// mylib.NewDict() translates to this
template <typename K, typename V>
Dict<K, V>* NewDict() {
return Alloc<Dict<K, V>>();
Expand All @@ -191,12 +197,14 @@ Dict<K, V>* NewDict(std::initializer_list<K> keys,
auto self = Alloc<Dict<K, V>>();
auto v = values.begin(); // This simulates a "zip" loop
for (auto key : keys) {
// note: calls reserve(), and maybe allocate
self->set(key, *v);
++v;
}

return self;
}
#endif

template <typename K, typename V>
void Dict<K, V>::reserve(int n) {
Expand Down Expand Up @@ -403,7 +411,7 @@ class DictIter {
// dict(l) converts a list of (k, v) tuples into a dict
template <typename K, typename V>
Dict<K, V>* dict(List<Tuple2<K, V>*>* l) {
auto ret = NewDict<K, V>();
auto ret = Alloc<Dict<K, V>>();
ret->reserve(len(l));
for (ListIter<Tuple2<K, V>*> it(l); !it.Done(); it.Next()) {
ret->set(it.Value()->at0(), it.Value()->at1());
Expand Down
16 changes: 12 additions & 4 deletions mycpp/gc_dict_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,25 @@
#include "mycpp/gc_mylib.h"
#include "vendor/greatest.h"

// Convenience function
template <typename K, typename V>
Dict<K, V>* NewDict() {
return Alloc<Dict<K, V>>();
}

GLOBAL_STR(kStrFoo, "foo");
GLOBAL_STR(kStrBar, "bar");

TEST test_dict_init() {
Str* s = StrFromC("foo");
Str* s2 = StrFromC("bar");

Dict<int, Str*>* d = NewDict<int, Str*>({42}, {s});
Dict<int, Str*>* d = Alloc<Dict<int, Str*>>(std::initializer_list<int>{42},
std::initializer_list<Str*>{s});
ASSERT_EQ(s, d->index_(42));

Dict<Str*, int>* d2 = NewDict<Str*, int>({s, s2}, {43, 99});
Dict<Str*, int>* d2 = Alloc<Dict<Str*, int>>(
std::initializer_list<Str*>{s, s2}, std::initializer_list<int>{43, 99});
ASSERT_EQ(43, d2->index_(s));
ASSERT_EQ(99, d2->index_(s2));

Expand Down Expand Up @@ -241,8 +249,8 @@ TEST test_dict_internals() {
StackRoots _roots6({&two});

auto dict3 =
NewDict<int, Str*>(std::initializer_list<int>{1, 2},
std::initializer_list<Str*>{kEmptyString, two});
Alloc<Dict<int, Str*>>(std::initializer_list<int>{1, 2},
std::initializer_list<Str*>{kEmptyString, two});
StackRoots _roots7({&dict3});

ASSERT_EQ_FMT(2, len(dict3), "%d");
Expand Down

0 comments on commit c415837

Please sign in to comment.