ooc-lang / rock

:ocean: self-hosted ooc compiler that generates c99
http://ooc-lang.org/
MIT License
401 stars 40 forks source link

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

Open alexnask opened 8 years ago

alexnask commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago
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.