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

Restrictions and more examples for the Include annotation #3452

Merged

Conversation

henrikt-ma
Copy link
Collaborator

Fixes #3451.

Opening in Draft state as the idea is to have a discussion here about how to resolve #3451.

chapters/functions.tex Outdated Show resolved Hide resolved
chapters/functions.tex Outdated Show resolved Hide resolved
chapters/functions.tex Outdated Show resolved Hide resolved
@HansOlsson
Copy link
Collaborator

When we add so many variants it might be best to also include the case of having a prototype directly in the included C-code.
(In terms of C-terminology a prototype is a "function declaration" in contrast to a "function definition".)

…notation

To make sense, I needed to put all semantics related to the annotation in the same place.
@henrikt-ma
Copy link
Collaborator Author

When we add so many variants it might be best to also include the case of having a prototype directly in the included C-code. (In terms of C-terminology a prototype is a "function declaration" in contrast to a "function definition".)

I tried to do this, but in order to make explain why one would like to do so, I found it necessary to have all semantics for the Include annotation in one place.

Regarding prototype vs function declaration, I agree that we should go for the standard terminology used in C. Should we do it as part of this PR?

chapters/functions.tex Outdated Show resolved Hide resolved
@HansOlsson
Copy link
Collaborator

When we add so many variants it might be best to also include the case of having a prototype directly in the included C-code. (In terms of C-terminology a prototype is a "function declaration" in contrast to a "function definition".)

I tried to do this, but in order to make explain why one would like to do so, I found it necessary to have all semantics for the Include annotation in one place.

Ok, but see comment about the latest addition.

Regarding prototype vs function declaration, I agree that we should go for the standard terminology used in C. Should we do it as part of this PR?

Better to have it later, and separately as it involves other lines as well.

…es.h

A replacement will be proposed in the form of a separate PR.
@henrikt-ma henrikt-ma marked this pull request as ready for review December 7, 2023 21:47
@henrikt-ma
Copy link
Collaborator Author

Removed Draft state as this seems to have converged enough for a language group decision.

@HansOlsson
Copy link
Collaborator

To me this makes sense, but since it adds new behavior I think it is good to get input from the group. Use emojis to vote - (left-bottom, closes day before phone meeting):

  1. Thumbs up: Ok to merge.
  2. Eyes: Need more discussion.
  3. Thumbs down: Don't specify these restrictions.

@HansOlsson HansOlsson self-requested a review January 12, 2024 14:18
Copy link
Collaborator

@HansOlsson HansOlsson left a comment

Choose a reason for hiding this comment

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

Seems ok.

@HansOlsson HansOlsson merged commit 7fe3b48 into modelica:master Jan 24, 2024
@henrikt-ma henrikt-ma deleted the include-annotation-restrictions branch January 29, 2024 07:24
@HansOlsson HansOlsson added the M37 For pull requests merged into Modelica 3.7 label May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
M37 For pull requests merged into Modelica 3.7
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Are different external functions allowed to Include non-static definitions of the same function?
2 participants