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

missing const checking for array of owned copy-init #14923

Open
mppf opened this issue Feb 19, 2020 · 0 comments
Open

missing const checking for array of owned copy-init #14923

mppf opened this issue Feb 19, 2020 · 0 comments

Comments

@mppf
Copy link
Member

mppf commented Feb 19, 2020

This program should fail to compile

class C { var x: int; }

proc main() {
  var X: [1..2] owned C?;
  X[1] = new owned C(1);
  X[2] = new owned C(2);

  const Y = X;

  const Z = Y;

  writeln(X);
  writeln(Y);
  writeln(Z);
}

because the const Z = Y does an init-copy from Y which is const. When copying the elements, the RHS will be set to nil (because owned = does that).

The array init-copy should consider the case that the element type has an init= / = that mutates the RHS.

PR #14887 attempted to fix it but that caused problems with shadow variables with the test test test/parallel/forall/reduce-intents/ri-a2-AoA.chpl and so PR #14922 removed the change.

Associated Future Test

test/types/records/const-checking/array-of-owned-const.chpl (added in PR #14998).

mppf added a commit that referenced this issue Feb 19, 2020
Un-do array init copy change

Changes in array initCopy from PR #14887 caused failures for the test
test/parallel/forall/reduce-intents/ri-a2-AoA.chpl in performance runs.
The problem is related to const-checking of shadow variables that I'm not
quite sure how to resolve at the moment.

This PR removes the intent changes for array initCopy.

Issue #14923 tracks the future work of fixing it (along with the new
future test/types/records/const-checking/array-of-owned-const.chpl)

While there, run the test that failed in performance testing in regular
testing too. Since full testing passed with the PR before it was merged,
this test is testing something different from others.

Trivial and not reviewed.

- [x] full local testing
mppf added a commit to mppf/chapel that referenced this issue Feb 27, 2020
mppf added a commit to mppf/chapel that referenced this issue Feb 27, 2020
mppf added a commit to mppf/chapel that referenced this issue Feb 28, 2020
mppf added a commit to mppf/chapel that referenced this issue Feb 28, 2020
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

1 participant