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

In a world with intents on de-tupled arguments, should the default intent be const? #26338

Open
bradcray opened this issue Dec 3, 2024 · 2 comments

Comments

@bradcray
Copy link
Member

bradcray commented Dec 3, 2024

Today, if a user uses argument based-detupling, like this:

proc myproc((x, y)) {

they are permitted to modify the formal arguments x and y within the body of the routine:

  x += 1;
  y += 1;
  writeln((x, y));  // prints (2, 3.3)
}

and these assignments don't change the actual argument [ATO]:

var tup = (1, 2.3);
myproc(tup);
writeln(tup);  // prints (1, 2.3)

In effect, this treats proc myproc((x, y)) similar to proc myproc(in (x, y)), though we don't currently support argument intents on de-tupled arguments like this.

In a world where we did, it seems as though it would be better to treat proc myproc((x, y)) like proc myproc(const (x, y)) by default, similar to other arguments and for similar reasons: To avoid the potential to mistakenly assume that the default acts like in vs. ref and potentially being surprised. It would also make proc myproc((x, y)) behave more like proc myproc(myTuple), where myTuple is const by default for a tuple actual.

This is obviously a breaking change, though it may still be a worthwhile one in that it arguably improves consistency and also wouldn't silently surprise anyone, since a program that relied on the current in-like behavior would get const errors in the new interpretation and would have an easy fix (add in to retain the old behavior).

@mppf
Copy link
Member

mppf commented Dec 3, 2024

This is obviously a breaking change, though it may still be a worthwhile one in that it arguably improves consistency and also wouldn't silently surprise anyone, since a program that relied on the current in-like behavior would get const errors in the new interpretation and would have an easy fix (add in to retain the old behavior).

IMO this is a bug ("missing const checking for de-tupling formals") and changing it is OK because it would be a bug fix.

@ty1027
Copy link

ty1027 commented Dec 3, 2024

I have read the manual page
https://chapel-lang.org/docs/language/spec/tuples.html#splitting-a-tuple-into-multiple-formal-arguments-in-a-function-call
and it has this example:

The function f defined as

proc f((x, (y, z))) {
  writeln((x, y, z));
}
is equivalent to the function g defined as

proc g(t) where isTuple(t) && t.size == 2 && isTuple(t(1)) && t(1).size == 2 {
  writeln((t(0), t(1)(0), t(1)(1)));
}
except without the definition of the argument name t.

This example explains that the two routines f and g are equivalent, so the reader might expect const to be attached on the argument of f (if the g has const by default).

In the case of sumOfSquares() in the nbody program of the BenchmarkGames site, I also imagined that if the routine is inline-ed and the user modifies the arguments inside the routine, then that modification might have unintentional side effects if the compiler uses the actual argument directly to evaluate the contents of the routine (without making temporary variables). So const by default may be safer for this kind of small routines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants