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.

Sign up for GitHub

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

Jump to bottom

Return record #3155

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

Return record #3155

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d0c1755
Enhancement to allow functions to return record values.
HansOlsson Apr 26, 2022
e5a8ee1
Clarify table as indicated in previous comment.
HansOlsson Apr 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions chapters/functions.tex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2229,8 +2229,8 @@ \subsubsection{Records}\label{records}
Arrays cannot be mapped.
\end{itemize}

Records are passed by reference (i.e.\ a pointer to the record is being
passed).
Records that are the return value of the C function are returned by value.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this should be allowed as I don't see that we should rely on any definition of CRec in the signature. See #3154.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't see a problem with returning structs by value, as it has been done in C since at least the 1980s.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This only works well if nested records are also stored as struct und not as pointer in the surrounding struct. Else the question about the memory management arises again.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not a concern about returning records by value in C in general, but a concern about using this mechanism in Modelica's external function protocol. The problem we have is that the external function should come with a prototype, and that prototype will have a definition of the record type that the tool generating the call to the external function cannot reasonably be expected to be aware of. (At least I don't want a solution where the tool needs to scan files included via the Include annotations in search of a the definition of the record used as the external function's return type.)

This test program demonstrates the problem:

struct A
{
  double x;
  int y;
};

struct B
{
  double x;
  int y;
};

struct A
f()
{
  struct A a;
  a.x = 0.5;
  a.y = 1;
  return a;
}

int
main()
{
  struct B b = f();
  return 0;
}

Clang rejects this with:

return_record.c:25:12: error: initializing 'struct B' with an expression of incompatible type 'struct A'
  struct B b = f();
           ^   ~~~

Beside the problem that the program is rejected by Clang, we have the potential inefficiency of returning big structs by value.

Just for clarity, this is the way I believe it needs to be done:

void
f(void* a)
{
  struct A* a_ = (struct A*)a;
  a_->x = 0.5;
  a_->y = 1;
}

int
main()
{
  struct B b;
  f((void*)(&b));
  return 0;
}

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @henrikt-ma

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That is horribly clunky.

First and foremost using void*-pointers is just asking for trouble - and #3198 gives a way of ensuring that the records are consistently defined.

For the possibility of return structs - what many/most/all(?) C-compilers internally do when you declare a function with a large struct as return value is to send in the return address to the function.

So, basically the C-compiler already do the same as this proposal, but automatically, type-safe, and only when needed. Having that as an option for users makes a lot more sense.

Other records are passed by reference (i.e.\ a pointer to the record is being passed).

\begin{example}
\begin{lstlisting}[language=modelica]
Expand Down Expand Up @@ -2283,10 +2283,11 @@ \subsection{Return Type Mapping}\label{return-type-mapping}
\lstinline!String! & \lstinline[language=C]!const char*! & \emph{Not allowed}\\
\lstinline!T[$\mathit{dim}_{1}$, $\ldots$, $\mathit{dim}_{n}$]! & \emph{Not allowed} & \emph{Not allowed} \\
Enumeration type & \lstinline[language=C]!int! & \lstinline[language=FORTRAN77]!INTEGER!\\
Record & See \cref{records} & \emph{Not allowed}\\
Record & \lstinline[language=C]!struct CRec! & \emph{Not allowed}\\
HansOlsson marked this conversation as resolved.
Show resolved Hide resolved
\hline
\end{tabular}
\end{center}
For the contents of \lstinline[language=C]!struct CRec! see \cref{records}.
HansOlsson marked this conversation as resolved.
Show resolved Hide resolved

The element type \lstinline!T! of an array can be any simple type as defined in \cref{simple-types} or, for C, a record type is returned as a value of the record type defined in \cref{records}.

Expand Down