-
Notifications
You must be signed in to change notification settings - Fork 64
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
C Generics #1681
C Generics #1681
Conversation
- Build Fixes - Allowing Template Types to be defined for in/out ports or state variables - Reordering definitions with includes Signed-off-by: mkhubaibumer <khubaib@magnition.io>
Basic implementation for Generic Reactors in C-Target are done We can run the following Generic Code for demonstration of this feature. These macros will be included to LF for user conveniance ``` target C { keepalive: true, // fast: true } preamble {= /// -- Start // All C-Generic Reactors will probably need these /// -- End =} reactor Connector<I, O>(latency:int(0)) { input in:I; output out:O; logical action delayed_sch; reaction(in) -> delayed_sch {= lf_schedule(delayed_sch, self->latency); =} reaction(delayed_sch) in -> out {= if (is_same_type(in->value, out->value)) { lf_set(out, in->value); } else { auto_t tmp = CALL_CONVERTOR(I, O, in->value); // If convertor function is defined this will be called lf_set(out, tmp); } =} } reactor A { output Aout:int; logical action post; timer t(0, 1sec); reaction(t) -> post {= lf_schedule(post, 2); =} reaction(post) -> Aout {= lf_set(Aout, rand()); =} } reactor B { input Bin:float; reaction(Bin) {= printf("got %f\n", Bin->value); =} } main reactor { a = new A(); w = new Connector<int, float>(latency=500); w2 = new Connector<char, long>(latency=500); b = new B(); a.Aout -> w.in; w.out -> b.Bin; } ``` Signed-off-by: mkhubaibumer <khubaib@magnition.io>
Basic implementation for Generic Reactors in C-Target are done We can run the following Generic Code for demonstration of this feature. ``` target C { keepalive: true, cmake-include: [ "../lib/lib.cmake" ], files: [ "../lib/convertor.h", "../lib/convertor.c" ] } preamble {= =} reactor Connector<I, O>(latency:int(0)) { input in:I; output out:O; logical action delayed_sch; reaction(in) -> delayed_sch {= lf_schedule(delayed_sch, self->latency); =} reaction(delayed_sch) in -> out {= if (is_same_type(in->value, out->value)) { lf_set(out, in->value); } else { auto_t tmp = CALL_CONVERTOR(I, O, in->value); lf_set(out, tmp); } =} } reactor A { output Aout:int; logical action post; timer t(0, 1sec); reaction(t) -> post {= lf_schedule(post, 2); =} reaction(post) -> Aout {= lf_set(Aout, rand()); =} } reactor B { input Bin:float; reaction(Bin) {= printf("got %f\n", Bin->value); =} } main reactor { a = new A(); w = new Connector<int, float>(latency=500); b = new B(); a.Aout -> w.in; w.out -> b.Bin; } ``` Complete Working example can be fetched from: https://github.com/MagnitionIO/LF_Collaboration/blob/main/CGenericsProposal/src/CGenerics.lf Signed-off-by: mkhubaibumer <khubaib@magnition.io>
Moved type-converter macro to utils Signed-off-by: mkhubaibumer <khubaib@magnition.io>
Moved type-converter macro to utils Signed-off-by: mkhubaibumer <khubaib@magnition.io>
…into c-templates
Updates for resolving Actions/StateVariables typenames to concrete types Updates to remove COrresponding tags for declerations Signed-off-by: mkhubaibumer <khubaib@magnition.io>
Defining template types in header can cause redefinitions in case of contained reactors using `#if defined/#undef/#define` construct can cause type collission as contained reactor can use the same template literals for different types than the parent reactor To solve this issue we decided to go for string substitution in header file to rersolve templated types to their respective concrete types and we'll still be using `#define` in scource files as the template types need to be defined as per instantiation in the source file Signed-off-by: mkhubaibumer <khubaib@magnition.io>
Basic Support for Contained Reactors Signed-off-by: mkhubaibumer <khubaib@magnition.io>
This commit fixes the issues with having multiple contained generic reactors Signed-off-by: mkhubaibumer <khubaib@magnition.io>
Update to support Templated Initialization of Generic Contained Reactor with `typeArgs` from Parent Reactor Signed-off-by: mkhubaibumer <khubaib@magnition.io>
Converted the Context Manager to Generic This required us to seperate the Templated-Types Pointers from their Tokens( * OR [] ) This commit fixes these issue and the GenericContextManager is now available in our Repo Signed-off-by: mkhubaibumer <khubaib@magnition.io>
Fix the case where Contained Reactor is initiated with a combination of Templated Types and Concrete Types Signed-off-by: mkhubaibumer <khubaib@magnition.io>
@mkhubaibumer and @petervdonovan: what is the status of this work? We don't want this PR to fall behind... |
The line I deleted here was doing nothing according to the compiler.
There is only one test, so this is easy. I think that adding a more thorough test suite and getting it to pass should be left outside the scope of this PR since this PR has lingered too long already and already provides useful functionality.
There are a few reasons for this change * the key is everywhere being computed from an Instantiation, so this change reduces the total amount of code * hash codes can occasionally collide, and I was not sure that this particular hashcode was very effective at minimizing collision probability. * but more importantly, the hash codes were being computed from information that does not uniquely identify the Instantiation. It was being computed from the name of the variable that holds the instance and from the name of the reactor class being instantiated. So if the same reactor class were instantiated and stored in a variable of the same name in multiple places, this could result in bugs. This change means that we are using physical equality to check the keys to the hashmap because we don't have our own IR and we can't override the hashcode and equals functions of Instantiation. But I think that's OK. After AST transformations, which should happen as upstream as possible, we are working with one single object representation of the program.
The tests were all passing, but it looked wrong as-is. Maybe the tests were passing because we were not using the feature that lets the user use types in a reaction body that relate to contained reactors.
We do not need it. We can obtain a TPR from an Instantiation when all the concrete types are immediately available, and when they are not immediately available, we the map does not help us.
I am still not sure that we want to maintain this generics-related utility file. Let us not worry about it until later.
Prefer to work on InferredTypes or Types rather than raw strings. Our code base, esp. the C generator, has had multiple problems involving ad hoc matching of raw strings.
It is usually recommended in the Java community to create objects specifically for the purpose at hand rather than using general tuple types.
This has only one smoke test, and I believe that some functionality related to generics that we want to work will not work. However I think we should merge this now rather than waiting because it touches many lines (mostly just with variable renamings, but still enough to make it time-consuming to resolve conflicts). |
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 agree with your assessment, @petervdonovan; this looks solid enough to merge, and we can fine tune later. Thanks for your your efforts, @mkhubaibumer and @petervdonovan!
org.lflang/src/org/lflang/generator/python/PythonGenerator.java
Outdated
Show resolved
Hide resolved
org.lflang/src/org/lflang/generator/c/TypeParameterizedReactor.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Marten Lohstroh <marten@berkeley.edu>
Co-authored-by: Marten Lohstroh <marten@berkeley.edu>
Support Generics on C-Target
Complete description is available on #1671