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

Make included function definitions static #74

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

henrikt-ma
Copy link

The purpose is to support generation of one C file per external function. See modelica/ModelicaSpecification#3451 for background.

This makes the external functions compatible with generation of one compilation unit for each external function.
@HansOlsson
Copy link
Collaborator

The purpose is to support generation of one C file per external function. See modelica/ModelicaSpecification#3451 for background.

As far as I understand the outcome of that PR I believe this change isn't needed (but still ok).

@henrikt-ma
Copy link
Author

I'd say it is needed because the same function definitions are included in multiple places. For instance,


and

Without the functions being static, this violates this rule:

The Include annotation shall be used in such a way that each external function can be handled in a separate translation unit.

@HansOlsson
Copy link
Collaborator

I would say that we need to redesign then.
It doesn't make sense to include three copies of the functions as static variants in the model (apart from being weird it is also fragile).

One solution would be have three different C-files (one per function) and just include the function that is needed at each place. That makes it straightforward to handle each of them in a separate translation unit - and prevents the fragile cross-dependencies.

@henrikt-ma
Copy link
Author

I don't mind the approach in #79. I wouldn't go as far as to call the approach with static definitions in a shared file weird, though, as I think it might appeal to some users who prefer to see their implementations side by side, and I can't really see the fragility of it as long as it is used systematically.

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.

2 participants