-
Notifications
You must be signed in to change notification settings - Fork 63
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
Environments in the C target #1772
Conversation
…resent fields in the enclave. Will move more info about the enclaves there
@petervdonovan @lhstrh @edwardalee I have now updated the tracing API also to deal with multiple environments. This PR and lf-lang/reactor-c#212 are now ready for review |
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.
Spot check looked good to me. Let's get this merged!
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 did not understand everything going on here, but I did not spot any important issues.
core/src/main/java/org/lflang/generator/c/CTriggerObjectsGenerator.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/lflang/generator/python/PythonGenerator.java
Outdated
Show resolved
Hide resolved
Thanks for your feedback! I have addressed it now. I think @petervdonovan raises an important point about the pollution of the namespaces. I have an issue on this here: lf-lang/reactor-c#237. I can commit to addressing this issue next. I would like to merge these two big PRs now and rather address the re-architecting of the platform-support in a separate PR. What do you think? |
I 100% agree. I do not want the namespace problems to hold up this PR and would be willing to help with them later |
Lets merge this, @lhstrh ? |
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.
LGTM (there is a comment staged from an earlier look at this, but I don't know what it is, so we'll see as soon as I hit the "approve" button).
I dont have permissions to merge the branch or add it to the merge queue. I can only keep updating the branch which triggers a new run of the tests. |
This PR is (mostly) about introducing the environment concept in reactor-c. There are also changes related to enclaves (mostly contained in
CEnclavedReactorTransformation.java
). This PR is mainly about:_lf_initialize_trigger_objects
so that we incoroprate the move from global variables mentioned in (2) to variables on the environment struct.Companion reactor-c PR: lf-lang/reactor-c#212