Skip to content

Implement In, InOut, Out argument passing intents #1807

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

Closed
certik opened this issue May 13, 2023 · 12 comments
Closed

Implement In, InOut, Out argument passing intents #1807

certik opened this issue May 13, 2023 · 12 comments

Comments

@certik
Copy link
Contributor

certik commented May 13, 2023

Here S is any type (i32, f64, Struct, Array, etc.).

In:

def f(x: In[S]) -> None:
   x[5] = 5 # Assignment not allowed

InOut:

def f(x: InOut[S]) -> None:
   x[5] = 5 # Allowed

Out:

def f(x: Out[S]) -> None:
   x[5] = 5 # Allowed

When we write just:

def f(x: S) -> None:
   x[5] = 5 # ?

Then we have to decide what the default intent is. The three options that make the most sense are In, InOut, Unspecified, all of which are supported in ASR. The Unspecified intent mean that it can be any of In, InOut, Out, and we don't know which one. This is problematic, because the caller then doesn't know if you do f(5) if 5 should be passed by value (for In), or by reference (for InOut / Out). In Fortran we have to support it in ASR, but in Python I would not allow it, I would always specify intent (implicitly or explicitly), so ASR is always exactly one of In, InOut or Out. I think that is better for optimizations and checking.

A good backwards compatible choice is that default of x: S will be x: In[S], so assignment is NOT allowed:

def f(x: S) -> None:
   x[5] = 5 # Assignment not allowed

Later, if we wanted, we can change the default to InOut, so assignment would be allowed and it would not break any existing code. However, given the fact that int can only be In (see below), for consistency defaulting everything to In seems like a good consistent choice.

I think being restrictive by default is good, and users opt-in for less restrictions.

Finally, there is one Python specific issue: the Python semantics only allows InOut and Out for composite types like Class, Array, List. It is not allowed for primitive types like int, float, which are passed "by value" in CPython, so they can only be "In". As a result, x: InOut[i32] is a compile time error.

Note: Fortran also allows unspecified intent, and "value" attribute. I think this allows to actually assign to the argument, without modifying it in the caller. We CAN later support this, but I would start with being restrictive and not allow Unspecified intent. It does not close the door to later add it. I've never used this feature in Fortran, and I always specify intent, I think the compiler should even enforce it by default. So I recommend in LPython to start with the above minimal design, always require In, InOut, Out as the only options in ASR, and if it is not specified in the Python source code, the LPython frontend will default to In and set In in ASR. Later it can be changed / extended, without breaking anything.

@Shaikh-Ubaid
Copy link
Collaborator

Finally, there is one Python specific issue: the Python semantics only allows InOut and Out for composite types like Class, Array, List. It is not allowed for primitive types like int, float, which are passed "by value" in CPython, so they can only be "In". As a result, x: InOut[i32] is a compile time error.

Here, do you mean for all primitive types (i32, f32, ...), x: InOut[primitive type] and x: Out[primitive type] should be compile time errors in lpython?

@certik
Copy link
Contributor Author

certik commented May 13, 2023

Here, do you mean for all primitive types (i32, f32, ...), x: InOut[primitive type] and x: Out[primitive type] should be compile time errors in lpython?

Yes. I think we have to, since in CPython you can't return anything into the caller via such arguments.

@dylon
Copy link

dylon commented May 13, 2023

Since CPython does not support passing by reference, would it be better to rename InOut/Out to Mutable and In to Immutable? The default behavior could be Immutable, requiring the customer to specify when he wants to mutate his parameters.

@Shaikh-Ubaid
Copy link
Collaborator

It seems, currently by default the function params are mutable and we already have Const type for immutable.

@certik
Copy link
Contributor Author

certik commented May 13, 2023

Interesting proposal to do Mutable/Immutable, I need to think about it more. Essentially Mutable is InOut. So we would not have a way to specify Out, I am not sure right now if it is needed, or InOut is all that is needed. The default should be immutable. The Const I think is meant for declaring variables, not so much for argument passing, although C/C++ uses it that way.

@certik
Copy link
Contributor Author

certik commented May 17, 2023

@Shaikh-Ubaid I think you closed this by a mistake, we still have to implement it.

@certik certik reopened this May 17, 2023
@Shaikh-Ubaid
Copy link
Collaborator

I sincerely apologize. Yes, it happened by mistake. It was entirely unintentional. I believe it occurred due to a misclick with my laptop trackpad while going through the details. I genuinely apologize for prematurely closing it without completing the necessary implementation steps. I will be careful in the future.

@certik
Copy link
Contributor Author

certik commented May 17, 2023

No worries! I figured it was by accident. You got me excited though as I thought for a second that you fixed it so quickly. :)

@Shaikh-Ubaid
Copy link
Collaborator

I thought for a second that you fixed it so quickly

I think we can get it working quickly. I had this commit from last week 4cce3e1. I submitted a PR here #1820.

@certik
Copy link
Contributor Author

certik commented May 17, 2023

Regarding Immutable (In) and Mutable (InOut): one avantage of Out over InOut is that the compiler can warn about uninitialized variables / members. The other advantage of Out is that the compiler can throw away the value/contents of the argument in the caller, since it won't be used/accessed inside the function, thus potentially allowing optimizations that InOut does not allow. So I think both InOut and Out is useful.

With our current design so far (if no intent is specified, then In is assumed), we can also introduce mutable/immutable by simply saying that "nothing" means immutable (=In), and x: Mut[i32[:]] is equivalent to x: InOut[i32[:]]. So you can either just think of InOut as Mut, or we can even allow that alias. So it seems the current design allows both approaches to coexist.

@Shaikh-Ubaid
Copy link
Collaborator

We currently support the intents In, Out and InOut. We can easily add alias Immutable (In) and Mutable (InOut). Is it safe to close this issue or shall we add the alias intents?

@certik
Copy link
Contributor Author

certik commented May 24, 2023

Let's do without the aliases for now and see how it goes. Thanks @Shaikh-Ubaid for fixing it!

@certik certik closed this as completed May 24, 2023
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

No branches or pull requests

3 participants