Moo like #4
Moo like #4
Conversation
meta.cc
Outdated
| for (MAGIC* mg = SvMAGIC(sv); mg; mg = mg->mg_moremagic) { | ||
| if (mg->mg_virtual == marker) { | ||
| return mg->mg_ptr; | ||
| PackageMeta find_package(HV* stash) { |
There was a problem hiding this comment.
Используй все-таки CAIXS_mg_findext.
meta.cc
Outdated
| void FieldMeta::activate(HV* obj) { | ||
| STRLEN f_len; | ||
| const char* f_name = SvPV(name, f_len); | ||
| STRLEN name_len; |
meta.cc
Outdated
|
|
||
| SV** ref = hv_fetch(fields, k_name, k_len, 0); | ||
| if (ref) return; | ||
| if (SvOK(default_value) && (!SvROK(default_value) || SvTYPE(SvRV(default_value)) != SVt_PVCV)) |
There was a problem hiding this comment.
If a simple scalar is provided, it will be inlined as a string.
Давай для совместимости сделаем так же.
meta.cc
Outdated
| SV** ret = hv_store(fields, k_name, k_len, field_sv, 0); | ||
| if (!ret) SvREFCNT_dec_NN(field_sv); | ||
| FieldMeta& field = fields[fields_sz]; | ||
| field.name = SvREFCNT_inc(hash_key); |
There was a problem hiding this comment.
Можно брать _NN версии.
xs/meta.h
Outdated
| void activate(SV* object); | ||
| HV* fields; | ||
| }; | ||
| typedef AV* PackageMeta; | ||
|
|
||
| void init_meta(); |
There was a problem hiding this comment.
А init_meta же больше не вызывается?
meta.cc
Outdated
| void PackageMeta::activate(SV *sv) { | ||
| if (!(sv && SvROK(sv))) return; | ||
| void activate(PackageMeta meta, SV *sv) { | ||
| if (!SvROK(sv)) croak("cannot activate non-reference"); |
There was a problem hiding this comment.
Тут же всегда корректный объект из конструктора, и проверки не нужны?
meta.cc
Outdated
| SV* field_sv = HeVAL(cur); | ||
| FieldMeta* field = find_field(field_sv); | ||
| if (!field) continue; | ||
| STRLEN field_len; |
There was a problem hiding this comment.
Аналогично комменту про utf8 выше - зачем использовать строковый апи без предпосчитанного хэша?
meta.cc
Outdated
|
|
||
| field->activate(hv); | ||
| if (SvTRUE(field.required)) { |
There was a problem hiding this comment.
Можно SvTRUE проверку вынести на этап install, и ставить оттуда жестко PL_sv_yes/PL_sv_no - и эта строчка станет сравнением с указателем.
meta.cc
Outdated
| if (!ref) croak("key '%s' is required", field_name); | ||
| return; | ||
| } else if (SvOK(field.default_value)) { | ||
| // ... |
meta.cc
Outdated
| if (field.required == &PL_sv_yes) { | ||
| SV** ref = hv_fetch(hv, field_name, field_len, 0); | ||
| if (!ref) croak("key '%s' is required", field_name); | ||
| if (SvOK(field.default_value)) { |
There was a problem hiding this comment.
SvOK упадет на NULL, а при этом ты используешь SvREFCNT_inc_simple для работы в default_value. Так оно NULL или не-NULL?
meta.cc
Outdated
| if (count != 1) croak("unexpected return from 'default': %d, expected: 1", count); | ||
|
|
||
| SV* new_val = POPs; | ||
| SvREFCNT_inc(new_val); |
meta.cc
Outdated
| int count = call_sv(fields->default_value, G_SCALAR | G_NOARGS); | ||
| SPAGAIN; | ||
|
|
||
| if (count != 1) croak("unexpected return from 'default': %d, expected: 1", count); |
There was a problem hiding this comment.
Я бы либо вообще убрал эту проверку (и брал просто 1й элемент со стека), либо сделал текст ошибки более понятным.
meta.cc
Outdated
| croak_sv(err); | ||
| } | ||
| } | ||
| else field.default_value = SvREFCNT_inc_simple_NN(default_value); |
There was a problem hiding this comment.
Скобочки в if/else блоке обязательны
No description provided.