-
Notifications
You must be signed in to change notification settings - Fork 100
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
Nested function types support #294
base: master
Are you sure you want to change the base?
Conversation
If I recall correctly I was getting tests passed when I tested it locally, on travis it shows lots of failed test cases with some wierd info |
The problem is the debugger sources aren't compiling correctly on Travis using this branch or master -- the tests never even run. However, they work perfectly on my machine. I've even tried a git clone into a new directory and using the I'm not sure whether this warning has anything to do with it:
@yanhick Any ideas on why this would fail on Travis, but not on a local workstation? |
You probably have a mismatch between your local config and Travis's. Maybe The description of the Travis build env is here: For the Java warning, I found this: Hope this helps ! 2015-06-08 14:35 GMT-07:00 Eric B notifications@github.com:
|
On my local machine, I always use Sun JDK 1.6. Travis doesn't have that. I'm thinking that this may, indeed, be a change to JDK 1.7. Maybe a security update that Travis has deployed? At any rate, to build with Java 1.7 on Travis we're going to have to play with the compiler settings. |
I find this doc about Travis build environnement update: There doesn't seem to be any change since April. Yes you could update the build to target Java 1.7 or alternately you could 2015-06-08 14:57 GMT-07:00 Eric B notifications@github.com:
|
AFAIK, Ubuntu is not licensed to distribute any Sun/Oracle JDK through its repositories. Oracle has discontinued JDK1.6 except for some large corporate licenses; it's no longer available on their web sites. |
You're right, I forgot about that, I always install openjdk and don't 2015-06-08 15:30 GMT-07:00 Eric B notifications@github.com:
|
} | ||
|
||
return true; | ||
return HaxeComponentType.typeOf(element) != HaxeComponentType.FIELD; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tend to leave the old code in place for these types of changes. I write them as separate returns when I need to break in the debugger on a certain return value, because you can't set a breakpoint on the return after the expression has been evaluated. The other pattern that I use is
ret = expr;
return ret;
My thinking for leaving them is that if I needed the returns separate for debugging in the past, then I'll probably need it again, and it doesn't make the code any harder to understand. It's just a style thing, though.
Yannick has fixed the Travis build for the moment by not regenerating the Java code for the debugger protocol. He's filed issue #305. Can you please merge master down into this branch and re-push. Once we get a clean compile, this is approved for merge. |
On second thought. Wait to merge until after we pull Carlos' change, which should be today if we don't find any more issues. |
@as3boyan Carlos' change has been pulled. You may proceed at your leisure. |
@boyan This branch was approved for merge a while back. Do you remember why it wasn't merged? Is it still valid? The build error was because 14.1.1 was no longer available on the Jetbrains site. The correct course of action is probably to merge master into this branch locally and then push it back up so that we can get a current build and test run. |
I don't remember, but I remember that we have some issue with nested function (now I can't even imagine how nested functions look) |
Conflicts: src/14.1/com/intellij/plugins/haxe/runner/debugger/HaxeDebugRunner.java
Note that the new tests I just added fail. This is because the golden masters haven't been properly created yet. And that is because I think that the parsing result is incorrect (and probably was incorrect before this change). This line: typedef MyFnPtr = String->Int->Void; creates this PSI:
Note how the FUNCTION_TYPE element contains a nested FUNCTION_TYPE element, instead of a series of TYPE_OR_ANONYMOUS elements. When processing the code looking for a function type, we won't see a proper function type, rather it will be interpreted as: typedef MyFnPtr = (String->Int)->Void; which is the wrong answer. The problem lies in these three lines in the BNF:
|
Nested function types support
#293