cappuccino / cappuccino

Web Application Framework in JavaScript and Objective-J
https://cappuccino.dev/
GNU Lesser General Public License v2.1
2.21k stars 333 forks source link

Many compiler fixes #1929

Closed mrcarlberg closed 11 years ago

mrcarlberg commented 11 years ago

I have made a lot of changes and fixes to the new compiler. We have been running it for the last month in our project and it feels very stable. I think it is time to merge these changes into master. Some of the changes are:

  1. Full gcc preprocess support with macros and preprocess directives like #if, #else, #endif etc. It can't build Cappuccino without gcc yet but we are slowly getting there.
  2. The compiler now works with IE8. Issue #1901.
  3. Fixed issue #1830 when return statement didn't have whitespace before expression.
  4. The compiler now generates all the code from the AST tree.
  5. The compiler is finally 100% pure javascript and doesn't use any preprocess macros anymore.
cappbot commented 11 years ago

Milestone: Someday. Label: #new. What's next? A reviewer should examine this issue.

aljungberg commented 11 years ago

Thanks, I'll take a look soon.

assignee=aljungberg milestone=0.9.7

cappbot commented 11 years ago

Assignee: aljungberg. Milestone: 0.9.7. Label: #new. What's next? A reviewer should examine this issue.

ahankinson commented 11 years ago

-#new +#accepted +Objective-J

cappbot commented 11 years ago

Assignee: aljungberg. Milestone: 0.9.7. Labels: #accepted, Objective-J. What's next? A reviewer should examine this issue.

aljungberg commented 11 years ago

This looks great. Well done. If you wrote any tests while working on this, maybe you could add them to Tests/Objective-J/Preprocessor/OutputTest.j.

mrcarlberg commented 11 years ago

I agree that your solution for the generate flag might be better but we have to alter it a little as we don't write to compiler.jsBuffer all the time.

Yes, 'this.generate' is always true.

I think we should just remove the generate flag and always generate the code in the future. I have not done this yet as I want it to be tested in the public so we don't find some cases where it won't work or cause problems. Then we don't need to worry about these at all.

We can just leave it as it is right now as I feel this is one step in a many step transformation of the compiler.

mrcarlberg commented 11 years ago

I'll add a test case to OutputTest.j

mrcarlberg commented 11 years ago

I have now added some very simple test cases for preprocess directives. I didn't know OutputTest.j existed so I will add more test cases here in the future.

I also changed the test case so it uses the new compiler instead of the old one :)

mrcarlberg commented 11 years ago

Does anyone know why the travis build is failing on CPDateFormatterTest? I can't reproduce it. I think it did pass before my last commit.

ahankinson commented 11 years ago

I can't reproduce it on my Mac, but I would wonder if it has something to do with the compiler running on rhino, not jsc

Just a guess though -- someone with linux might want to test it.

mrcarlberg commented 11 years ago

I tried to compile and run the test cases with rhino on my Mac and I still don't get any errors. Is there a way to redo the travis build?

ahankinson commented 11 years ago

I've found that doing it on a Linux box usually results in the same behaviour. I was going to try it on one of our VMs, but I haven't found the time yet.

If you just want to kick off another Travis build, you can just add another commit to this PR -- that should re-do it.

mrcarlberg commented 11 years ago

Ok, I'll merge it with the latest master. That should trigger the travis build

aparajita commented 11 years ago

@aljungberg Are we ready to merge this?

aljungberg commented 11 years ago

@mrcarlberg hmm why revert the test case? @aparajita not quite yet.

mrcarlberg commented 11 years ago

I did the revert as the travis build was failing. I can't reproduce the failure on my Mac, even when I run with rhino. OutputTest.j is a very special test case that adds test cases dynamic. It might be the problem.

I'm going to try to find out why but it might take some time.

I think we can merge what we have and merge in the test cases later when I have solved this.

aljungberg commented 11 years ago

I'll poke around with the tests tomorrow if I have a moment and if it looks good I'll merge then regardless of what Travis thinks.

mrcarlberg commented 11 years ago

I just found out that travis runs the test cases in another order then when I run them on my machine. On my Mac OutputTest runs almost last along with the rest of the Objective-J tests. Travis runs the Objective-J tests first so OutputTest is in the beginning.

When I now reverse sort the file list in the jake file I can reproduce this failure on my computer :)

I'll be back!

mrcarlberg commented 11 years ago

Ok, I really hate the use of:

var a = 3,
    b = 4,
    c = b;

It is so easy to write:

var a = 3;
    b = 4,
    c = b;

And the code does something totally different and it is very hard to find the problem. I think it is much better to write:

var a = 3;
var b = 4;
var c = b;

I agree that it looks better to use the commas. Maybe I can let the compiler do warnings but it is a little tricky to know when.

mrcarlberg commented 11 years ago

From my point of view this is now ready for merge

aparajita commented 11 years ago

Ok, I really hate the use of:

var a = 3,
b = 4, c = b; It is so easy to write:

var a = 3; b = 4,
c = b;

It is so easy to use capp_lint, which catches this.

mrcarlberg commented 11 years ago

Still the frameworks have had a lot of these and might still have. I know I have fixed 20 or more since I started on the new compiler 6 month ago.

mrcarlberg commented 11 years ago

I ran capp_lint on the whole Cappuccino code base and found some more.

Now, it should be ready to merge.

ahankinson commented 11 years ago

+#ready-to-commit

cappbot commented 11 years ago

Assignee: aljungberg. Milestone: 0.9.7. Labels: #accepted, #ready-to-commit, Objective-J. What's next? The changes for this issue are ready to be committed by aljungberg.

aparajita commented 11 years ago

That means we haven't been running capp_lint on new commits, which is our fault.

aljungberg commented 11 years ago

So at this stage what is it that requires the gcc preprocessor still? #include statements?

aparajita commented 11 years ago

So at this stage what is it that requires the gcc preprocessor still? #include statements?

Yes. We can get rid of the #include Foundation.h in AppKit. The only other includes are in Objective-J/Includes.js as a way of concatenating the final file, but that can easily be accomplished in another way.

I will take care of this in the Node branch.

aljungberg commented 11 years ago

Merged. Great work with preprocessor evaluation and the new generate from AST mechanism. Thanks!

+#fixed

cappbot commented 11 years ago

Assignee: aljungberg. Milestone: 0.9.7. Labels: #fixed, Objective-J. What's next? This issue is considered successfully resolved.

aparajita commented 11 years ago

Yes, absolutely awesome work! And perfect timing, I need all of these features for the Node branch.

mrcarlberg commented 11 years ago

Yes, there should be work arounds for all '#includes'.

What we need is a way to pass in macros in to the compiler (like the -D argument in gcc).

Now the compiler is part of the Objective-J framework and the framework is using a lot of preprocessor macros. The easiest ways should be to extract the compiler from the framework and use it to preprocess all the files in the framework.

aparajita commented 11 years ago

Yes, the compiler needs to be a separate component. What is involved in doing that?

ahankinson commented 11 years ago

Thanks!

+#fixed

cacaodev commented 11 years ago

Great work martin !

Can you add the "allow class override in browser" feature ? Ideally, the requirements would be this: Allow class override if

If it's impossible or too difficult to determine 2/, we can use a special compiler directive, defined in the browser, that enables directly the feature.