Project

General

Profile

Bug #10621

no parent in rb_data_type_t

Added by Hanmac (Hans Mackowiak) over 5 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Normal
Target version:
ruby -v:
ruby 2.2.0dev (2014-12-19 trunk 48891) [x86_64-linux]
[ruby-core:66969]

Description

rb_data_type_t does need to have a parent type otherwise it cant bind objects anymore that have a tree style ClassTree like in C++ or simulated with gtk

in C++ like Class B and Class C does inherit both Class A, cant be ported with ruby anymore because with that Class A cant have its own datatype anymore because the datatype check does break the inheritence. (id did worked before because it did had parent

Updated by nobu (Nobuyoshi Nakada) over 5 years ago

  • Description updated (diff)
  • Target version set to 2.2.0
  • Backport changed from 2.0.0: UNKNOWN, 2.1: UNKNOWN to 2.0.0: DONTNEED, 2.1: DONTNEED

I've been lazy to revert r48647, but incline to do it now.
How do you think, _ko1?

Updated by ko1 (Koichi Sasada) over 5 years ago

I'm okay to revert for avoiding parent.

However, I'm negative because parent abstraction is not good idea for rb_data_type_t.

Reasons:

  • You can make such checking code in few lines with data field.
  • mark, free, memsize doesn't care about parent (and difficult to care about parent. it depends on situations)
  • rb_data_type_t is not an official API (so that README.ext doesn't refer on it)

Updated by nobu (Nobuyoshi Nakada) over 5 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

Applied in changeset r48962.


ruby.h: parent in rb_data_type_t

  • ruby.h (rb_data_type_t): revert r48647 and revise parent member. [ruby-core:66969] [Bug #10621]

Updated by normalperson (Eric Wong) over 5 years ago

ko1@atdot.net wrote:

  • rb_data_type_t is not an official API (so that README.ext doesn't refer on it)

Can we make it official?
I consider using it in some exts to avoid problems like [Bug #10296]

Updated by Hanmac (Hans Mackowiak) over 5 years ago

Koichi Sasada wrote:

I'm okay to revert for avoiding parent.

However, I'm negative because parent abstraction is not good idea for rb_data_type_t.

Reasons:

  • You can make such checking code in few lines with data field.

No does not work because its about rb_check_typeddata, how else should a function know if the object i have can be turned to the given type? it cant be done nicely with the data field because i need to do all checks the rb_check_typeddata function does by my self before too

  • mark, free, memsize doesn't care about parent (and difficult to care about parent. it depends on situations)

And thats a good point for me, because i have objects of the class that should not always be deleted because it might cause segfaults, thats why i have two data_type for the same class (i dont know of a better way to do that)

  • rb_data_type_t is not an official API (so that README.ext doesn't refer on it)

as Eric did said that the old API with unchecked is getting deprecated, the new one should be added to the README.ext, same for the scan_kargs function

Updated by ko1 (Koichi Sasada) over 5 years ago

No does not work because its about rb_check_typeddata, how else should a function know if the object i have can be turned to the given type? it cant be done nicely with the data field because i need to do all checks the rb_check_typeddata function does by my self before too

Why does not it work?

And thats a good point for me, because i have objects of the class that should not always be deleted because it might cause segfaults, thats why i have two data_type for the same class (i dont know of a better way to do that)

Could you explain more? I can't understand your sitation. Sample code are very welcome.

as Eric did said that the old API with unchecked is getting deprecated, the new one should be added to the README.ext, same for the scan_kargs function

To make official API, we remove parent filed because it is ad-hoc, not considered field.

At first, original T_DATA structure is not deprecate. Most of case it is enough, I think.
And I agree that it is not enough for some case and it is good to become deprecate.

So let's start to discuss about it.
Your case will be nice example to discuss to make it official API.

Updated by Hanmac (Hans Mackowiak) over 5 years ago

Koichi Sasada wrote:

No does not work because its about rb_check_typeddata, how else should a function know if the object i have can be turned to the given type? it cant be done nicely with the data field because i need to do all checks the rb_check_typeddata function does by my self before too

Why does not it work?

it might be able to work, but with that i need to write the rb_typeddata_is_kind_of function by myself and might not work so well might also be to much complicated

And thats a good point for me, because i have objects of the class that should not always be deleted because it might cause segfaults, thats why i have two data_type for the same class (i dont know of a better way to do that)

Could you explain more? I can't understand your sitation. Sample code are very welcome.

i wrap T* arg objects into my ruby objects, but sometimes i get a const T* arg, not wanting to waste extra memory, i wrap them too, but them because objects of that class should be freed from ruby it does try to free that one too, and that causes a double free because the library does delete them too (links below)

as Eric did said that the old API with unchecked is getting deprecated, the new one should be added to the README.ext, same for the scan_kargs function

To make official API, we remove parent filed because it is ad-hoc, not considered field.

At first, original T_DATA structure is not deprecate. Most of case it is enough, I think.
And I agree that it is not enough for some case and it is good to become deprecate.

So let's start to discuss about it.

Your case will be nice example to discuss to make it official API.

my binding: https://github.com/Hanmac/rwx
the binded lib: https://github.com/wxWidgets/wxWidgets

where that const pointer does come from: https://github.com/wxWidgets/wxWidgets/blob/master/include/wx/gdicmn.h#L1020
and there i register the data types: https://github.com/Hanmac/rwx/blob/master/ext/main.cpp#L47

i hope you guys understand my code and how i try to get the datatype from/for a ruby class and other stuff

for some classes (that does inherit from wxWindow and some EventHandler) i also added a GC protection that prevent ruby from removing the objects as long as the C++ objects does survive (might not be one of my best coding parts but its better than nothing)

i dont have a problem if you guys can find ways to make my code better, for faster typing i am accessible on irc.freenode.net as hanmac too (mostly in #ruby)

Also available in: Atom PDF