Re: [PATCH v4] Add support for creating new types from the Python API

[Tom Tromey <tom@tromey.com>]

>>>>> "Matheus" == Matheus Branco Borella <dark.ryu.550@gmail.com> writes:

Matheus> Thanks for the review, I've got a few questions and things to add before
Matheus> I submit the v5, if that's okay.

Thanks.  I'm sorry about the delay on this, pinging was the right thing
to do.

>> Is the none case really possible?
>> It might be better to just throw an exception from the constructor or
>> during argument validation or something like that.

Matheus> Most of these fall under the same response, so I'll just reply to them
Matheus> all at once.

Matheus> When I was writing this patch, I had the following in mind:
Matheus>  1st - This patch was first written before GDB switched to C++17, so I
Matheus>        had no access to std::optional<>.
Matheus>  2nd - I felt like throwing an exception over doing the `->valid()`
Matheus>        check explicitly would be less clear about my intent for people
Matheus> 	   reading the code.

Matheus> The design of `type_storage_owner` follows from those, and I don't feel
Matheus> like changing it to use std::option<> or exceptions would be much of an
Matheus> improvement in readability.

Matheus> Would it really be that much of an improvement?

I took another look at the patch and I think I understand.  I agree,
this seems fine.

>> I think the uses of this could probably use TYPE_ALLOC instead.

Matheus> Isn't that only valid for `struct type`? I don't think I follow. Some of
Matheus> the allocations (and I'm pretty sure at least one has to) happen before
Matheus> the call to `init_*_type`.

Yeah, I see.  It still feels like some of this could be using
type_allocator or be pushed there, but it's not a big deal, I wouldn't
worry about it.

thanks,
Tom