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

The way rock constructs class information can lead to data races in seemingly unrelated code #1003

Open
alexnask opened this issue Jun 30, 2016 · 3 comments

Comments

@alexnask
Copy link
Collaborator

Here is a sample _class() function the backend generates lifted from the following class:

Foo: class {
    init: func
    bar: func
}
test__FooClass *test__Foo_class(){
    static _Bool __done__ = false;
    static test__FooClass class = 
    {
        {
            {
                {
                    .instanceSize = sizeof(test__Foo),
                    .size = sizeof(void*)
                },
                .__defaults__ = (void*) test__Foo___defaults___impl,
                .__destroy__ = (void*) lang_types__Object___destroy___impl,
            },
        },
        .bar = (void*) test__Foo_bar_impl,
    };
    lang_types__Class *classPtr = (lang_types__Class *) &class;
    if(!__done__){
        classPtr->super = (lang_types__Class*) lang_types__Object_class();
        __done__ = true;
        classPtr->name = (void*) lang_String__makeStringLiteral("Foo", 3);
    }
    return &class;
}

Obviously, this function is not thread safe.

Proposed generated code:

const test__FooClass test__Foo_class = {
        {
            {
                {
                    .instanceSize = sizeof(test__Foo),
                    .size = sizeof(void*),
                    .name = lang_String__makeStringLiteral("Foo", 3)
                },
                .__defaults__ = (void*) test__Foo___defaults___impl,
                .__destroy__ = (void*) lang_types__Object___destroy___impl,
            },
        },
        .bar = (void*) test__Foo_bar_impl,
    };

Also, anywhere test__Foo_class() would appear, we replace it with &test__Foo_class.

@fasterthanlime
Copy link
Collaborator

I remember a reason for it being as ugly as it is right now. I would've loved to generate the code you propose instead but I'm pretty sure meta-classes are going to implode if you go that route. Test well :)

@alexnask
Copy link
Collaborator Author

@fasterthanlime
I kind of assume the worst is going to happen too :P
Will be reporting on this thread, I'm sure it will be interesting.

@alexnask
Copy link
Collaborator Author

alexnask commented Jun 30, 2016

C:\rock\sdk\lang\Character.ooc:172:25: note: (near initialization for 'lang_Character__Char_class.__super__.__super__.__super__.name')
C:\rock\sdk\lang\Character.ooc:194:29: error: initializer element is not constant

Was expecting an error of this nature, need to reconsider the generated code.

EDIT:
Alternative codegen: Generate the above without the '.name' field, initialize '.name' in the module load.

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

No branches or pull requests

2 participants