ooc-lang / rock

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

Fields initialized as array literals in a class' defaults generate invalid C code #912

Closed alexnask closed 9 years ago

alexnask commented 9 years ago

Here are the problematic C sources (come from sdk/os/native/TerminalWin32.ooc):

void os_native_TerminalWin32__TerminalWin32___defaults___impl(os_native_TerminalWin32__TerminalWin32* this) {
    os_Terminal__TerminalHandler___defaults___impl((os_Terminal__TerminalHandler*) this);
    this->bg = 0;
    this->fg = 8;
    this->colors = (lang_Memory__memcpy(__os_native_TerminalWin32_arrLit1.data, __os_native_TerminalWin32_ptrLit2, 9 * ((lang_types__Class*)lang_Numbers__Int_class())->size), __os_native_TerminalWin32_arrLit1);
    #line 25 "/Users/amos/Dev/ooc/rock/sdk/os/native/TerminalWin32.ooc"
    _lang_array__Array __os_native_TerminalWin32_arrLit1 = _lang_array__Array_new(lang_Numbers__Int, 9);
    #line 25 "/Users/amos/Dev/ooc/rock/sdk/os/native/TerminalWin32.ooc"
    lang_Numbers__Int* __os_native_TerminalWin32_ptrLit2 = (lang_Numbers__Int[]) { 0, 12, 10, 14, 9, 13, 11, 7, 31 };
}

The following code is produced by the following ooc code:

version(windows) {
// Some winapi binding happening here...
    TerminalWin32: class extends TerminalHandler {
        bg := Color black
        fg := Color white

        init: func

        /* Color codes */
        // Here is the issue (this should also be static or even better, an enum)
        colors := [
            0 , // black
            12, // red
            10, // green
            14, // yellow
            9 , // blue
            13, // magenta
            11, // cyan
            7 , // grey
            31  // white
        ]
        // Implements the Terminal API from hereon...
    }
}

I'm also getting the "undefined reference to `GC_beginthreadex'" error when I correct the C code manually, I've gotten around that one before though, I'll figure it out.

Any idea why the array literal is defined after the actual variable?
Perhaps some issue with array members in version blocks?
I can't really find another testcase atm, I will come up with one if I manage to get rock to compile though.

fasterthanlime commented 9 years ago

Hmm did I upload a bad 0.9.11-prealpha1 bootstrap? Will have to check.

os/Terminal could definitely use a bit of a cleanup. As for GC_beginthreadex, I suppose it's a boehm gc lib clash again.. if it was picking up the one compiled from rock's Makefiles, it should have all the threading support we need. (Hmm.. or not, maybe it does need --enable-threads=win32 explicitly for windows).

The wrong order for literals is something I've seen in the middle of my cleanups, but I think it was before the 0.9.10 release. Might have been after and I'm mistaken, though.

alexnask commented 9 years ago

By manually switching around the C code and using the instructions from here, I managed to get c_rock, I do get 'C compiler failed on module os/native/TerminalWin32 from sdk, bailing out' on make self though, so I assume the C code generated is still invalid, will tkae a look now.

(By the way, c_rock -V output is rock 0.9.11-head codename sapporo, built on Sun Jul 12 15:53:51 2015)

fasterthanlime commented 9 years ago

Ah yes, actually the release was made here: https://github.com/fasterthanlime/rock/commit/3f04249b22195ab72c8d4848d6596dee1f348100

Looking at the history for ArrayLiteral, it looks like it hasn't been touched after July 10? Would that mean that it is still broken?

Actually, that's completely plausible, since the travis tests don't run on Windows, the windows code doesn't even compile.

horasal commented 9 years ago

A simple test:

Foo: class{
    test := [1, 2, 3]
    init: func
}

describe("array literial should be correctly unwrapped in default",|| Foo new())
fasterthanlime commented 9 years ago

@zhaihj if at all possible, I prefer tests to do some runtime checking as well. That test might very well compile but put garbage inside the array, and we wouldn't know (and it's happened before..)

alexnask commented 9 years ago

As a temporary workaround, I applied @zhaihj's patch and commented out the failing code in TerminalWin32.ooc, then made rock with c_rock (OOC=bin/c_rock make self)

And then, of course, uncommented the code and ran a regular make self

fasterthanlime commented 9 years ago

Closed by #915!