Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undefined behavior #3

Open
russelltg opened this issue May 4, 2016 · 12 comments
Open

Undefined behavior #3

russelltg opened this issue May 4, 2016 · 12 comments

Comments

@russelltg
Copy link

Hey just so your know.....

http://scarff.id.au/blog/2009/c-multi-character-character-constants/

you are relying on lots of UB to make this library unique...I mean it works under like ALL compilers but still at least put it in the readme.

@r-lyeh-archived
Copy link
Owner

Yep, it does not break portability but I'd be better to explicit tell so indeed
Thanks for the suggestion :)

@russelltg
Copy link
Author

I really want to use this library, but really want to have more determinate
component IDs (for networking etc) so I am thinking hard of another
implementation, I'll suggest it as a pull request when I have time.

I really like your implementation of most everything else, it is a super
cool concept! I like that there is no world-like object.

-Russell

On Thu, May 5, 2016 at 2:12 AM r-lyeh [email protected] wrote:

Yep, it does not break portability but I'd be better to explicit tell so
indeed
Thanks for the suggestion :)


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#3 (comment)

@r-lyeh-archived
Copy link
Owner

you can use enums as well :)

@russelltg
Copy link
Author

Good point, but I would rather not break extensibility...I want people to
be able to confidently add components without clashes.

I'll work it out, I have an idea in mind, but you might not like it because
it has a macro in it...

On Fri, May 6, 2016 at 12:42 AM r-lyeh [email protected] wrote:

you can use enums as well :)


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#3 (comment)

@r-lyeh-archived
Copy link
Owner

r-lyeh-archived commented May 6, 2016

A few more ideas:

kult::component<'vel2', vec2> velocity; // current, but swap in template (if needed)
enum {
    VEL2
};
kult::component<VEL2, vec2> velocity; // portable
kult::component<'v','e','l','2', vec2> velocity; // glue in template
kult::component<SWAP_ENDIAN('vel2'), vec2> velocity; // endian swap in macro

@russelltg
Copy link
Author

While learning a little about Boost.Spirit, they use a really cool tecnique to do this exact thing--this is perfect, no macros, no UB, no hashing that is relying on chance.

The idea is to just have a quickly forward-declared type instead of the unsigned.

Potential example

kult::component<struct vel2_id, vec2> velocity;

Simple, beautiful. If you like this I am willing to create a pull request implementing this.

The only one minus with this is you loose your runtime unsigned, which might mean you need typeid or similar.......

@r-lyeh-archived
Copy link
Owner

r-lyeh-archived commented May 10, 2016

haha, such a cool find indeed!

yep switching the template<type,T> to template<typename,typename.> in component class plus the new return typeid(name).name(); in component::name() did the trick.

this is how the demo1.cc sample looks after switching to struct args

obj1 misses description field
obj2 has description field
{       struct Name: obj1,
        struct Position: 1,
}
{       struct Name: obj2,
        struct Position: 2,
        struct Description: this is our first object,
}
{}

it seems to work very well despite the breakage in the (portable) serialization (named fields are not portable anymore)

if there are workarounds for this, i'd like to know :)

@russelltg
Copy link
Author

Yeah, I have a workaround. This generates unique typeids:

unsigned next_typeid(){
    static unsigned next = 0;
    return next++;
}

template<typename T>
unsigned get_typeid() {
    static unsigned my_typeid = next_typeid();
    return my_typeid;
}

check it out here

@r-lyeh-archived
Copy link
Owner

r-lyeh-archived commented May 11, 2016

Well, it seems it depends now on the initialization order, which might be more dangerous than the fourcc codes solution even.

On the other hand, this would not fix the named fields bug in the serialization either.

Let me know if I'm mistaken :)

Feel free to fork the repo and commit all changes you need to do.
We'll discuss the pros and contras and review the issues, but I cannot guarantee that I am going to merge the changes unless they are a win-win solution.
Thanks :)

@russelltg
Copy link
Author

Of course, and yes your are right it depends on order, which is pretty bad,
somehow missed that :|.

I'll get back to you.

On Wed, May 11, 2016 at 2:45 AM, r-lyeh [email protected] wrote:

Well, it seems it depends now on the initialization order, which might be
more dangerous than the fourcc codes solution even.

On the other hand, this would not fix the named fields bug in the
serialization either.

Let me know if I'm mistaken :)

Feel free to fork the repo and commit all changes you need to do.
We'll discuss the pros and contras and review the issues, but I cannot
guarantee that I am going to merge the changes unless they are win-win
solution.
Thanks :)


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#3 (comment)

@r-lyeh-archived
Copy link
Owner

👍

@r-lyeh-archived
Copy link
Owner

I dunno where your last comment is. Gone?
Btw, this might be handy: https://github.com/irrequietus/typestring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants