Further cleanup

[edwardalee]

This cleanup pass moves a renames many functions to separate user-facing header files (e.g. reactor.h, schedule.h) from internal header files (e.g. reactor_common.h, reactor_threaded.h). A major goal is as a first step towards getting Doxygen docs to give us useful pages, making function names more uniform, cleaning up header files, and making internal functions that are not any part of an API static. This also removes some obsolete function declarations that had no implementations.

There is a companion PR in lingua-franca.

It may be easier to view the resulting header files rather than the diff when reviewing.

This work is not complete and there are still some inconsistent function names. The main focus of this PR was on reactor.h. The functions defined there can be viewed as complete.

[petervdonovan]

I found no problems in this PR. The only comments that I left are more high-level comments about the diff and notes to help other reviewers understand the diff.

[erlingrj]

What is our convention for selecting the name for a function that "does something" between the following:

1. do_something
2. lf_do_something
3. _lf_do_something

[cmnrd]

I was wondering the same. In particular, I noticed that https://github.com/lf-lang/lingua-franca/pull/2210/files removes the _ prefix from several functions.

I think the policy used to be that the _ prefix is used on functions that are visible in reaction bodies, but that are not supposed to be used directly by the user. Does this PR ensure that the renamed functions (_ prefix removed) are not visible in reaction bodies?

[edwardalee]

What is our convention for selecting the name for a function that "does something" between the following:

1. do_something
2. lf_do_something
3. _lf_do_something

Good question. I think the original intent was that do_something should only be used for static functions, where the scope is limited to the file. lf_do_something should be used for user-facing functions, such as those used in reaction bodies. And _lf_do_something should be used for things that can't be static because they are used in multiple files, but they are functions that a user would never invoke.

The problem I ran into is that it's not always clear which functions a user might want to invoke, and this could change over time. For example, should it be lf_new_reactor or _lf_new_reactor? lf_allocate or _lf_allocate?

I've been opting for doing away with the leading underscore when there is any doubt, but I wonder whether we should do away with it completely? Whether something is user facing, I think, should be determined by which header file declares it. If it's declared in reactor.h, then it is user facing and is in scope for reactions. If it is declared in reactor_common.h, then it is not in scope for reactions. If we organize the Doxygen docs well, all of this could be made very clear.

An advantage of doing away with the underscores altogether is that we can change our minds later relatively easily. Right now, _lf_schedule_at_tag is not a user-facing function, but maybe we will want it to be later. If we remove the leading underscore now, then the change is painless: Just move its declaration to reactor.h.

So my proposal is:

1. do_something for static functions (scope limited to the current file).
2. lf_do_something for everything else.

This PR does not make this change, but maybe we can do is part of the formatting PR? Or as a separate PR in rapid succession?

Edward

[cmnrd]

So my proposal is:

1. do_something for static functions (scope limited to the current file).
2. lf_do_something for everything else.

This PR does not make this change, but maybe we can do is part of the formatting PR? Or as a separate PR in rapid succession?

If we truly ensure that the internal API does not leak into the public API via some hidden dependencies, then yes, I think this would be feasible. Having an additional prefix has the advantage that we clearly label the functions. So, also when the function is used, we know whether it originates from a public or a private API. This is not strictly required though and I am fine with either solution.

The separation between public user-facing API and internal API is a great outcome of this PR. I think we should make it more explicit, by moving all headers that are user-facing in a single location and adding this location to cmake using the PUPLIC interface, while we should add all internal header locations using the PRIVATE interface. Then, we can be sure that only the public headers are visible in LF code.

[erlingrj]

This sounds like a good strategy. In a separate PR we will make a pass on the API and see if we can split it up into a public and a private API using CMake. I will finish going through this PR then :)

[cmnrd]

I agree that we can address this in a later PR. I opened #360 to keep track of the action items that we are aware of.

[erlingrj]

This looks very good. I guess we would like to move a lot more into the api/ folder? E.g. lf_time_* functions, lf_tag* functions? Or will we make a difference between functions that are purely user-facing and those that are also used internally by the runtime? (This is a topic for the next PR I guess)

[cmnrd]

I think we will need to different api folders, one user facing (in lf code) and one internally. My hope, however, is that we don't need an explicit internal directory. In a sense, the headers defined by the modules should define their API. A separate API module would only be needed if we have circular dependencies between modules, which hopefully we can avoid (but that is unclear).