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

Replace generated include folder with src-gen/include #7

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

petervdonovan
Copy link
Contributor

@petervdonovan petervdonovan commented Mar 1, 2024

To see how this change would look for users, see the changes to our tests here.

This is motivated by lf-lang/lingua-franca#2203.

This is an RFC because it could in principle be a breaking change. In fact, our tests would have compiled successfully even if the changes to the tests that were made in lf-lang/lingua-franca#2223 had not been made; however, it is possible to imagine that such a change in the layout of files that users are expected to access and read could break some code. At the very least, it is a change to the user experience, so I would like to make sure that we can come to a reasonably final decision on this that is unlikely to change again in a later release.

I propose that we aim to merge this RFC before March 7 so that we can resolve issue no. 2203, which I perceive as a serious issue. Update: Like #8, this is now indefinitely on a backlog.

@petervdonovan petervdonovan requested a review from cmnrd March 1, 2024 00:09
# Rationale and alternatives
[rationale-and-alternatives]: #rationale-and-alternatives

One alternative is to put these headers in a sibling directory of the `src-gen` directory called `include-gen`, and to either add that directory to the include path or copy it into src-gen and put the copied directory on the include path. The disadvantage of this alternative is that it violates the principle that generated files will go in src-gen; this principle is simple and easy to explain, so it is worth keeping.
Copy link
Member

Choose a reason for hiding this comment

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

Another take is that a directory that ends with -gen (such as src-gen but also fed-gen) contains generated code. If that is the principle, then this alternative solution is not a departure from it. I kind of like the idea of keeping things that are user-facing in separate directory.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we actually have fed-gen? Sure, we could have multiple -gen directories, but why not place everything in a single directory where we have full control. As Peter says, it's easy to describe to users and also it makes cleaning much simpler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I kind of like the idea of keeping things that are user-facing in separate directory.

This is taken into account in #8.

it makes cleaning much simpler

This is taken into account in #8.

it's easy to describe to users

Unfortunately, #8 would be complex to describe to users.

Copy link
Member

Choose a reason for hiding this comment

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

We have fed-gen because it contains generated Lingua Franca source code, which src-gen doesn't. Moreover, fed-gen has a src-gen directory with in it with generated target code, just like the root-level src-gen directory. This sounds like a pretty reasonable explanation to me. I understand that all if this could be changed, but it will cost us time, so we should only do this if there is something to gain from it. What advantage would it yield?

Copy link
Contributor

@cmnrd cmnrd Mar 13, 2024

Choose a reason for hiding this comment

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

Well, what we place in fed-gen is also generated code, the only difference is the language. Why does that difference matter?

How exactly is the include (or potential include-gen) actually user facing? Currently, I don't see the difference compared to the other generated code and in particular to reactor-c, all of which we place in src-gen. The user only interacts with this code by writing reactions, right? If that is the case, why should we treat it differently from the other generated code?

What advantage would it yield?

By using nested gen, build, and install directories, instead of many different ones on the top-level we gain parity between targets. We can identify a small set of directories that we create. This simplifies communication to the user, as we can state precisely which directories we create and which we potentially delete on a clean. In fact, I would advocate for introducing only a single directory, which we could call target or something similar. Inside this directory, we can then more freely define the structure we want, and it would be ok to also use different structures for different targets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have a strong opinion about this, but to answer the question:

How exactly is the include (or potential include-gen) actually user facing?

It includes type definitions and function prototypes that the user has to implement. The user has to #include the header file in their C code. Although the user could in principle read the LF code and figure out what the header file is going to contain, in practice I expect that the easiest thing will be for the user to just read the generated header file because they are already familiar the process of writing source files that implement things that are defined in header files. The header files may also be hooked into whatever editor support the user has for their C source files.

We could try to liberate the user from having to work directly with the header files but I consider that to be an unnecessary disruption to whatever their usual workflow probably is for writing C code.

@cmnrd
Copy link
Contributor

cmnrd commented Mar 1, 2024

This addresses the situation where we actually generate source files. However, the C++ target creates an include directory, alongside build, bin and share as part of the target compiler flow. The code that is located in src-gen gets compiled in build and then installed to the project directory, where cmake automatically places things in the bin, include and share directories. This is the same as if you would install in the system root, or in /usr, but using a prefix that points to the project root. How do we address this? Should we introduce an install directory, and install there? Then the bin directory would move, though.

This actually hints at another problem. What if the project has a bin directory created by the user that creates relevant scripts (like actually our LF repo does). Currently, we would also simply delete that.

@petervdonovan
Copy link
Contributor Author

This addresses the situation where we actually generate source files. However, the C++ target creates an include directory, alongside build, bin and share as part of the target compiler flow. The code that is located in src-gen gets compiled in build and then installed to the project directory, where cmake automatically places things in the bin, include and share directories. This is the same as if you would install in the system root, or in /usr, but using a prefix that points to the project root. How do we address this? Should we introduce an install directory, and install there? Then the bin directory would move, though.

This actually hints at another problem. What if the project has a bin directory created by the user that creates relevant scripts (like actually our LF repo does). Currently, we would also simply delete that.

I find this argument compelling. These problems seem interrelated enough and simple enough to address that it seems justified to open a competing RFC (#8) that has a wider scope. I am not sure which RFC is better, but I think that the competing RFC addresses both of the discussions here at the cost of being more complex to implement.

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.

3 participants