Reserved C keyword problems #365

Open
alexnask opened this Issue Feb 11, 2012 · 23 comments

Comments

Projects
None yet
6 participants
Collaborator

alexnask commented Feb 11, 2012

Some reserved C keywords are not detected by rock and thus compilation fails at the C level. Keywords I have detected until now are: float, void.
Should be an easy fix, will do that later :)

On the other hand, reserved keywords are not taken into account in a for loop declaration. For example, rock does not mark the following code as invalid:

import structs/ArrayList

for(char in [1,2,3] as ArrayList) {
    char toString() println()
}

Wich fails at C compile time.

Member

nddrylliog commented Feb 12, 2012

Aw yeah, C keyword checking is kind of shaky. I'm pretty sure it can be improved upon :) (and even done in a much better way in oc)

Collaborator

alexnask commented Feb 13, 2012

By the way, should these symbols really be blocked from being used in ooc code? Correct me if I'm wrong but as I recall, it has been a goal of yours to make ooc independent from C, while this shows the profound links the current implementation has with C. In my honest opinion, this should not be the case but rather every backend should handle code generation to avoid similar problems. For example, I think that it would be more reasonable to allow these names and when the code is generated, let the C backend mangle them. This would also make ooc more open to other implementations as compiler/target language specific restraints will be less apparent.

Tl;dr: Why not just mangle those reserved keywords at code generation?

Member

nddrylliog commented Feb 13, 2012

That's at the cost of readability of the generated C code, but I think we're way past this point anyway, so yes, I'd be in favor of letting the backends mangle and not reserve C keywords in ooc code.

Collaborator

alexnask commented Feb 13, 2012

Excellent :)
I may work on a couple of bugs on rock tomorrow so this will be on my todo list.

Member

nddrylliog commented Feb 13, 2012

I think refactoring rock to allow that is an undertaking not to be underestimated! Keep in mind that if, for example, you escape 'char' to 'char_C' or the like, you'll have name collisions with a variable named 'char_C' to begin with.

The simple solution is to use a char combo that you can't have in ooc identifiers, but that doesn't exist right now. What you could do is escape all '_' in ooc identifiers to '__' in C identifiers, so that one couldn't obtain 'char_C' from an ooc identifier. Makes sense?

Collaborator

alexnask commented Feb 13, 2012

Yes this could be a possible solution to the problem. Thanks for the idea :-)
I will see what I can do. What I am thinking is to have a list of the reserved C keywords, like there is now and add some kind of identifier processing method in the C backend, witch will replace _ with __ and mangle the identifier if it is parte of the reserved keywords. It would also be an option to generate a random identifier but not without risk (there could still be this identifier "clash" problem)
Anyway, I don't quite remember the way rock generates the C code, apart from some really general ideas behind it, so I will see and adapt.

Member

nddrylliog commented Feb 13, 2012

rock generates the C code directly from the ooc AST, or to be clearer, a post-processed version of the ooc AST. As a result, this makes it necessary for the ooc AST and the C language to map more or less cleanly. That's why we have so many tree manipulation routines.

However, in oc, we have an ooc AST which is then turned into a C AST, allowing vastly different representations and a cleaner handling of ooc in general.

For rock, there are a few helpers to write identifiers, I guess you'll have to look in every corner to not miss one. Again, if we had an Identifier class or something (like I had to do in my school Scala subset compiler project) maybe this would be easier to handle.

Collaborator

alexnask commented Feb 14, 2012

Also, another issue to consider is that ooc offers the option to unmangle variable definitions. What should be the proper behaviour in this case? An error at parse-time, at code generation or just make the cnaughty abckend ignore this directive when reserved keywords are involved?

Member

nddrylliog commented Feb 14, 2012

I think using 'unmangle' with a reserved C keyword should probably yield an error in cnaughty yes.

unmangle is an ugly hack anyway, there should probably be a better way to achieve the same effect (ie. not have it as an ooc keyword, because it's C-specific.)

Collaborator

alexnask commented Feb 14, 2012

Ok then I will start working on this after I've finished up with another issue.
The unmangled keyword does give some interesting possibilities (like the stuff that is done to interface rock with nagaqueen), while it is true it is implementation-specific but I don't see how we could replace it.

Collaborator

alexnask commented Feb 14, 2012

Also apparently at the moment rock only tests if variable declarations have the name of a reserved keyword, witch would be fine without the existence of unmangled and if the for loop variable declaration was tested too, witch are the cases that can cause compilation to fail I have discovered up until now. I guess the for loop variable gets transformed into a variable declaration later on, so the checks necessary on the backend should not be that difficult to implement. Starting :D

Collaborator

alexnask commented Feb 14, 2012

I've started the changes but I am segfaulting for some reason... Anyway, I will continue later today or tomorrow :)

Member

nddrylliog commented Feb 14, 2012

Looking nice. rock segfaults can be a bit irritating. Do you know all the necessary gdb/recompilation/safe_rock voodoo? I'd assume yes since you've already fixed so many bugs :)

Collaborator

alexnask commented Feb 14, 2012

I've used my share fare of gdb, plus I think fredreichbier has put out a tool that unmangles ooc symbols in gdb, this will be very useful :D I don;t know anything about safe_rock though :-/

Member

nddrylliog commented Feb 14, 2012

So apparently I hadn't declared the valid? function as static but after doing so rock segfaults on self-compiling >.<

See that's what safe_rock is about. When you're meddling with powers that mighty, you want to keep a safe copy of rock around.

So the workflow goes a little bit like this. At first you have a safe rock (say, master on my repo), then you do

make backup

Which copies bin/rock to bin/safe_rock
Then you can go crazy hacking, and use

make safe

Instead of make self to compile your crazy modified rock with the safe rock. What it does in reality is something like OOC=bin/safe_rock make self, only shorter.

That'll save you a lot of headaches :)

Collaborator

alexnask commented Feb 16, 2012

Just a quick update: My had something to do with static arrays (ArrayLists's ? ), changing the reservedWords and reservedHashes variables to global fixed the issue. Now, I seem to have broken generic arguments (compilation fails at C level) but I think it's ebcause I haven't made any changes to VariableAccess writing witch I will do promptly.

Contributor

rofl0r commented May 4, 2012

i havent read everything here but i think for consistency any OOC symbol should be mangled to something like ooc_var

Collaborator

duckinator commented May 12, 2012

I agree with @rofl0r. Basically just prepend 'ooc_' before what we currently use. It should, by it's very nature, handle reserved C keywords without issue.

Member

nddrylliog commented May 13, 2012

We could just add o_, solves the same problem only shorter.

Btw, OOC_FROM_C doesn't exist anymore in conspiracy, so we don't give a shit about short names anymore.

Collaborator

duckinator commented May 14, 2012

Apparently I misunderstood this with my last comment.

Regardless, adding any sort of prefix to local variables helps with this issue, whether it be ooc_, o_, var_, or whatever.

ahamid added a commit to ahamid/rock that referenced this issue Jun 4, 2013

ahamid added a commit to ahamid/rock that referenced this issue Jun 4, 2013

ahamid added a commit to ahamid/rock that referenced this issue Jul 15, 2013

Contributor

davidhesselbom commented Jul 29, 2014

I think this is related, so I'll just post this here instead of creating a new issue:

    PointA: cover {
        x, y: Float
        init: func@ (=x, =y)
        // This gives "error: expected identifier or ‘(’ before ‘union’".
        // Union is a keyword in C and so cannot be used for method names,
        // but the name should be Point__union something, so there shouldn't be a problem.
        // Compiler bug?
        union: func (other: This) -> This {
            This new(0.0f, 0.0f)
        }
    }

    PointB: cover {
        x, y: Float
        init: func@ (=x, =y)
        // Using a suffix circumvents but does not solve the problem
        union: func ~point (other: This) -> This {
            This new(0.0f, 0.0f)
        }
    }
Collaborator

fasterthanlime commented Jul 29, 2014

@davidhesselbom look at the generated code, it's the vtable that's causing an issue:

struct _issue365__PointAClass {
    struct _lang_types__ClassClass __super__;
    void (*init)(issue365__PointA*, lang_Numbers__Float, lang_Numbers__Float);
    /* problem's here */
    issue365__PointA (*union)(issue365__PointA, issue365__PointA);
    void (*__cover_defaults__)(issue365__PointA*);
};


struct _issue365__PointBClass {
    struct _lang_types__ClassClass __super__;
    void (*init)(issue365__PointB*, lang_Numbers__Float, lang_Numbers__Float);
    issue365__PointB (*union_point)(issue365__PointB, issue365__PointB);
    void (*__cover_defaults__)(issue365__PointB*);
};

rock is being too lax imho, it should just forbid using 'union' like that - or prefix it like talked beforehand in this thread.

Contributor

davidhesselbom commented Jul 30, 2014

@fasterthanlime the generated code gives me dyslexia, that's why I'm writing my code in ooc (sexy syntax > dyslexia). But yes, I saw the generated code.

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