Skip to content
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

add _XOPEN_SOURCE in header file to get the right offset in eager strategy #84

Merged
merged 2 commits into from
Feb 11, 2015

Conversation

kikofernandez
Copy link
Contributor

No description provided.

@kaeluka
Copy link
Contributor

kaeluka commented Feb 11, 2015

have you talked to @albertnetymk about adding the _XOPEN_SOURCE lines? have you come to an agreement? the commit looks good, but i'm hesitant to merge it as albert seems to think that the line should not go in...

@albertnetymk
Copy link
Contributor

We just discussed this. This commit shall get us to the state that both coroutine and future are working properly on mac and linux. The reason why I claimed xopen is not needed here is that in lazy version (which is WIP) ucontext info is completely hidden in the runtime, and no userspace code would see it.

kaeluka pushed a commit that referenced this pull request Feb 11, 2015
add _XOPEN_SOURCE in header file to get the right offset in eager strategy
@kaeluka kaeluka merged commit 53cce3b into new-ponyrt Feb 11, 2015
@@ -24,6 +24,7 @@ generate_header A.Program{A.etl = A.EmbedTL{A.etlheader}, A.functions, A.classes
IfNDefine "HEADER_H" $
Concat $
HashDefine "HEADER_H" :
(HashDefine "_XOPEN_SOURCE 800") :
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to get some discussion, shouldn't this line be without parenthesis to follow the surrounding coding style?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are absolutely right. I'll fix it in another commit

@albertnetymk
Copy link
Contributor

Next time, we should wait for a while before merging. After all, PR is meant for code review & discussion. @EliasC For the style in Haskell file, I think so. For the commented C code, if (0) is kept there so that we could easily, sort of, switch back and forth. The printf line should be removed.

@kaeluka
Copy link
Contributor

kaeluka commented Feb 11, 2015

What's "a while"?

@kaeluka kaeluka deleted the new-ponyrt-eager branch February 11, 2015 15:06
@albertnetymk
Copy link
Contributor

Don't know exactly how long. Some comments provided by @EliasC should be applied, and rebase should get rid of them, but we didn't do that because insufficent time is provided before merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants