Skip to content

Commit 1dd8dca

Browse files
jabraham17jabraham17
authored
Change the ref-maybe-const deprecation on foralls into an unstable warning (#23526)
Changes the warning for an implicit ref intent on arrays in foralls into an unstable warning, which is off by default. After offline discussion, concerns raised in #23488 and #23229, and concerns raised by Arkouda developers, we decided to soften our stance on this change for the release. ## Summary of changes - add check for unstable flag in `compiler/resolution/lowerIterators.cpp` where the warning is thrown - updated warning message to indicate the feature is unstable - changed the wording in the language evolution document to reflect these changes - moved `test/unstable/ref-maybe-const-forall-intent.chpl` to the `unstable` tests folder - updated submitted perf tests .good files and perfkeys accordingly - test/studies/shootout/submitted/binarytrees3.chpl - test/studies/shootout/submitted/knucleotide3.chpl - test/studies/shootout/submitted/knucleotide4.chpl - test/studies/shootout/submitted/revcomp3.chpl - test/studies/shootout/submitted/revcomp5.chpl - test/studies/shootout/submitted/revcomp8.chpl - test/studies/shootout/submitted/spectralnorm.chpl - test/studies/shootout/submitted/spectralnorm2.chpl - test/studies/shootout/submitted/mandelbrot.chpl - test/studies/shootout/submitted/mandelbrot3.chpl ## Testing - paratest with futures - paratest with no futures + gasnet - local testing of all modified tests - local testing with `-performance` of all modified perf tests [Reviewed by @mppf]
2 parents 9378c40 + 50dbfb4 commit 1dd8dca

22 files changed

+37
-59
lines changed

‎compiler/resolution/lowerIterators.cpp

+6-2
Original file line numberDiff line numberDiff line change
@@ -3083,9 +3083,10 @@ void maybeIssueRefMaybeConstWarning(ForallStmt* fs, Symbol* sym, std::vector<Cal
30833083
for (auto ce: allCalls) {
30843084
// if a call sets the symbol, warn
30853085
if (callSetsSymbol(sym, ce)) {
3086+
// checking for --warn-unstable done in checkForallRefMaybeConst
30863087
USR_WARN(fs,
3087-
"inferring a default intent to be 'ref' is deprecated - "
3088-
"please add an explicit 'ref' forall intent for '%s'",
3088+
"inferring a 'ref' intent on an array in a forall is unstable"
3089+
" - in the future this may require an explicit 'ref' forall intent for '%s'",
30893090
sym->name);
30903091
break;
30913092
}
@@ -3094,6 +3095,9 @@ void maybeIssueRefMaybeConstWarning(ForallStmt* fs, Symbol* sym, std::vector<Cal
30943095

30953096

30963097
static void checkForallRefMaybeConst(ForallStmt* fs, std::set<Symbol*> syms) {
3098+
// bail out here if we shouldn't be warning for this forall
3099+
if (!shouldWarnUnstableFor(fs)) return;
3100+
30973101
// collect all SymExpr used in the iterand of the forall so we dont warn for them
30983102
std::vector<SymExpr*> allIterandSymExprs;
30993103
for_alist(expr, fs->iteratedExpressions()) {

‎doc/rst/language/evolution.rst

+16-11
Original file line numberDiff line numberDiff line change
@@ -60,15 +60,20 @@ The default intent for arrays and records
6060
*****************************************
6161

6262
In version 1.32, arrays and records now always have a default intent of
63-
``const``. This means that if arrays and records are modified inside of a loop
64-
body, a function, or a task body they must use a ``ref`` intent. This also
65-
means that record methods which modify their implicit ``this`` argument must
66-
also use a ``ref`` intent. Previously, the compiler would treat these types as
67-
either ``const ref`` intent or ``ref`` intent, depending on if they were
68-
modified. This change was motivated by improving the consistency across types
69-
and making potential problems more apparent.
70-
71-
Consider the following code segment, which contains a ``forall`` statement
63+
``const``. This means that if arrays and records are modified inside of a
64+
function, a ``coforall``, a ``begin``, or ``cobegin``, they must use a ``ref``
65+
intent. This also means that record methods which modify their implicit
66+
``this`` argument must also use a ``ref`` intent. Previously, the compiler would treat
67+
these types as either ``const ref`` intent or ``ref`` intent, depending on if
68+
they were modified. This change was motivated by improving the consistency
69+
across types and making potential problems more apparent.
70+
71+
Since there is a lot of user code relying on modifying an outer array, the
72+
corresponding change for ``forall`` is still under discussion. As a result, it
73+
will not warn by default, but modifying an outer array from a ``forall`` might
74+
not be allowed in the future in some or all cases.
75+
76+
Consider the following code segment, which contains a ``coforall`` statement
7277
which modifies local variables. Prior to version 1.32, this code compiled and
7378
worked without warning.
7479

@@ -78,7 +83,7 @@ worked without warning.
7883
const myDomain = {1..10};
7984
var myArray: [myDomain] int;
8085
81-
forall i in 2..9 with (ref myInt) {
86+
coforall i in 2..9 with (ref myInt) {
8287
myInt += i;
8388
myArray[i] = myArray[i-1] + 1;
8489
}
@@ -99,7 +104,7 @@ which can be a source of subtle bugs. In 1.32, the loop is written as:
99104
const myDomain = {1..10};
100105
var myArray: [myDomain] int;
101106
102-
forall i in 2..9 with (ref myInt, ref myArray) {
107+
coforall i in 2..9 with (ref myInt, ref myArray) {
103108
myInt += i;
104109
myArray[i] = myArray[i-1] + 1;
105110
}

‎test/deprecated/ref-maybe-const-forall-intent.good

-6
This file was deleted.

‎test/studies/shootout/submitted/binarytrees3.good

-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
binarytrees3.chpl:13: In function 'main':
2-
binarytrees3.chpl:38: warning: inferring a default intent to be 'ref' is deprecated - please add an explicit 'ref' forall intent for 'stats'
31
stretch tree of depth 11 check: 4095
42
1024 trees of depth 4 check: 31744
53
256 trees of depth 6 check: 32512

‎test/studies/shootout/submitted/binarytrees3.perfkeys

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
real
2-
verify:3: stretch tree of depth \d+\t check: 8388607
2+
verify:1: stretch tree of depth \d+\t check: 8388607
33
verify: long lived tree of depth \d+\t check: 4194303

‎test/studies/shootout/submitted/knucleotide3.good

-1
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ knucleotide3.chpl:76: In function 'calculate':
4242
knucleotide3.chpl:88: warning: 'map.items' is deprecated. Consider 'map.keys' to iterate over keys or 'map.values' to iterate over values.
4343
knucleotide3.chpl:69: called as calculate(data: [domain(1,int(64),one)] uint(8), param nclSize = 18) from function 'writeCount'
4444
knucleotide3.chpl:49: called as writeCount(data: [domain(1,int(64),one)] uint(8), param str = "GGTATTTTAATTTATAGT") from function 'main'
45-
knucleotide3.chpl:100: warning: inferring a default intent to be 'ref' is deprecated - please add an explicit 'ref' forall intent for 'toNum'
4645
A 30.279
4746
T 30.113
4847
G 19.835

‎test/studies/shootout/submitted/knucleotide3.perfkeys

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
real
2-
verify:66:GG 3.902
3-
verify:72:893 GGTATTTTAATTTATAGT
2+
verify:65:GG 3.902
3+
verify:71:893 GGTATTTTAATTTATAGT

‎test/studies/shootout/submitted/knucleotide4.good

-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ knucleotide4.chpl:75: In function 'calculate':
4848
knucleotide4.chpl:87: warning: 'map.items' is deprecated. Consider 'map.keys' to iterate over keys or 'map.values' to iterate over values.
4949
knucleotide4.chpl:68: called as calculate(data: [domain(1,int(64),one)] uint(8), param nclSize = 18) from function 'writeCount'
5050
knucleotide4.chpl:48: called as writeCount(data: [domain(1,int(64),one)] uint(8), param str = "GGTATTTTAATTTATAGT") from function 'main'
51-
knucleotide4.chpl:99: warning: inferring a default intent to be 'ref' is deprecated - please add an explicit 'ref' forall intent for 'toNum'
5251
A 30.279
5352
T 30.113
5453
G 19.835

‎test/studies/shootout/submitted/knucleotide4.perfkeys

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
real
2-
verify:72:GG 3.902
3-
verify:78:893 GGTATTTTAATTTATAGT
2+
verify:71:GG 3.902
3+
verify:77:893 GGTATTTTAATTTATAGT

‎test/studies/shootout/submitted/mandelbrot.pbm

-139 Bytes
Binary file not shown.

‎test/studies/shootout/submitted/mandelbrot3.pbm

-418 Bytes
Binary file not shown.

‎test/studies/shootout/submitted/revcomp3.good

-4
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,6 @@ revcomp3.chpl:15: warning: reader with a 'kind' argument is deprecated, please u
88
revcomp3.chpl:17: warning: openfd is deprecated, please use the file initializer with a 'c_int' argument instead
99
revcomp3.chpl:17: warning: writer with a 'kind' argument is deprecated, please use Serializers instead
1010
revcomp3.chpl:55: warning: inferring a default intent to be 'ref' is deprecated - please use an explicit 'ref' intent for the argument 'buf'
11-
revcomp3.chpl:55: In function 'revcomp':
12-
revcomp3.chpl:71: warning: inferring a default intent to be 'ref' is deprecated - please add an explicit 'ref' forall intent for 'buf'
13-
revcomp3.chpl:79: warning: inferring a default intent to be 'ref' is deprecated - please add an explicit 'ref' forall intent for 'buf'
14-
revcomp3.chpl:45: called as revcomp(buf: [domain(1,int(64),one)] uint(8)) from function 'main'
1511
>ONE Homo sapiens alu
1612
CGGAGTCTCGCTCTGTCGCCCAGGCTGGAGTGCAGTGGCGCGATCTCGGCTCACTGCAAC
1713
CTCCGCCTCCCGGGTTCAAGCGATTCTCCTGCCTCAGCCTCCCGAGTAGCTGGGATTACA

‎test/studies/shootout/submitted/revcomp3.perfkeys

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
real
2-
verify:16666685:AACATTACAGGTAATGATAA
2+
verify:16666681:AACATTACAGGTAATGATAA

‎test/studies/shootout/submitted/revcomp5.good

-4
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,6 @@ revcomp5.chpl:22: warning: reader with a 'kind' argument is deprecated, please u
88
revcomp5.chpl:24: warning: openfd is deprecated, please use the file initializer with a 'c_int' argument instead
99
revcomp5.chpl:24: warning: writer with a 'kind' argument is deprecated, please use Serializers instead
1010
revcomp5.chpl:64: warning: inferring a default intent to be 'ref' is deprecated - please use an explicit 'ref' intent for the argument 'buf'
11-
revcomp5.chpl:64: In function 'revcomp':
12-
revcomp5.chpl:71: warning: inferring a default intent to be 'ref' is deprecated - please add an explicit 'ref' forall intent for 'buf'
13-
revcomp5.chpl:79: warning: inferring a default intent to be 'ref' is deprecated - please add an explicit 'ref' forall intent for 'buf'
14-
revcomp5.chpl:54: called as revcomp(buf: [domain(1,int(64),one)] uint(8), lo: int(64), hi: int(64)) from function 'main'
1511
>ONE Homo sapiens alu
1612
CGGAGTCTCGCTCTGTCGCCCAGGCTGGAGTGCAGTGGCGCGATCTCGGCTCACTGCAAC
1713
CTCCGCCTCCCGGGTTCAAGCGATTCTCCTGCCTCAGCCTCCCGAGTAGCTGGGATTACA

‎test/studies/shootout/submitted/revcomp5.perfkeys

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
real
2-
verify:16666685:AACATTACAGGTAATGATAA
2+
verify:16666681:AACATTACAGGTAATGATAA

‎test/studies/shootout/submitted/revcomp8.good

-3
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,6 @@ revcomp8.chpl:80: warning: inferring a default intent to be 'ref' is deprecated
1717
revcomp8.chpl:146: warning: inferring a default intent to be 'ref' is deprecated - please use an explicit 'ref' intent for the argument 'buff'
1818
revcomp8.chpl:146: warning: inferring a default intent to be 'ref' is deprecated - please use an explicit 'ref' intent for the argument 'seq'
1919
revcomp8.chpl:96: warning: inferring a default intent to be 'ref' is deprecated - please add an explicit 'ref' task intent for 'seq'
20-
revcomp8.chpl:33: In function 'main':
21-
revcomp8.chpl:36: warning: inferring a default intent to be 'ref' is deprecated - please add an explicit 'ref' forall intent for 'pairCmpl'
22-
revcomp8.chpl:62: warning: inferring a default intent to be 'ref' is deprecated - please add an explicit 'ref' forall intent for 'buff'
2320
>ONE Homo sapiens alu
2421
CGGAGTCTCGCTCTGTCGCCCAGGCTGGAGTGCAGTGGCGCGATCTCGGCTCACTGCAAC
2522
CTCCGCCTCCCGGGTTCAAGCGATTCTCCTGCCTCAGCCTCCCGAGTAGCTGGGATTACA

‎test/studies/shootout/submitted/spectralnorm.good

-8
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,4 @@ spectralnorm.chpl:35: warning: inferring a default intent to be 'ref' is depreca
22
spectralnorm.chpl:27: warning: inferring a default intent to be 'ref' is deprecated - please use an explicit 'ref' intent for the argument 'tmp'
33
spectralnorm.chpl:43: warning: inferring a default intent to be 'ref' is deprecated - please use an explicit 'ref' intent for the argument 'Atv'
44
spectralnorm.chpl:27: warning: inferring a default intent to be 'ref' is deprecated - please use an explicit 'ref' intent for the argument 'AtAv'
5-
spectralnorm.chpl:35: In function 'multiplyAv':
6-
spectralnorm.chpl:36: warning: inferring a default intent to be 'ref' is deprecated - please add an explicit 'ref' forall intent for 'Av'
7-
spectralnorm.chpl:28: called as multiplyAv(v: [domain(1,int(64),one)] real(64), Av: [domain(1,int(64),one)] real(64)) from function 'multiplyAtAv'
8-
spectralnorm.chpl:17: called as multiplyAtAv(v: [domain(1,int(64),one)] real(64), tmp: [domain(1,int(64),one)] real(64), AtAv: [domain(1,int(64),one)] real(64))
9-
spectralnorm.chpl:43: In function 'multiplyAtv':
10-
spectralnorm.chpl:44: warning: inferring a default intent to be 'ref' is deprecated - please add an explicit 'ref' forall intent for 'Atv'
11-
spectralnorm.chpl:29: called as multiplyAtv(v: [domain(1,int(64),one)] real(64), Atv: [domain(1,int(64),one)] real(64)) from function 'multiplyAtAv'
12-
spectralnorm.chpl:17: called as multiplyAtAv(v: [domain(1,int(64),one)] real(64), tmp: [domain(1,int(64),one)] real(64), AtAv: [domain(1,int(64),one)] real(64))
135
1.274224116

‎test/studies/shootout/submitted/spectralnorm.perfkeys

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
# file: spectralnorm-submitted.dat
22
real
3-
verify:13: 1.274224153
3+
verify:5: 1.274224153

‎test/studies/shootout/submitted/spectralnorm2.good

-8
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,4 @@ spectralnorm2.chpl:35: warning: inferring a default intent to be 'ref' is deprec
22
spectralnorm2.chpl:27: warning: inferring a default intent to be 'ref' is deprecated - please use an explicit 'ref' intent for the argument 'tmp'
33
spectralnorm2.chpl:43: warning: inferring a default intent to be 'ref' is deprecated - please use an explicit 'ref' intent for the argument 'Atv'
44
spectralnorm2.chpl:27: warning: inferring a default intent to be 'ref' is deprecated - please use an explicit 'ref' intent for the argument 'AtAv'
5-
spectralnorm2.chpl:35: In function 'multiplyAv':
6-
spectralnorm2.chpl:36: warning: inferring a default intent to be 'ref' is deprecated - please add an explicit 'ref' forall intent for 'Av'
7-
spectralnorm2.chpl:28: called as multiplyAv(v: [domain(1,int(64),one)] real(64), Av: [domain(1,int(64),one)] real(64)) from function 'multiplyAtAv'
8-
spectralnorm2.chpl:17: called as multiplyAtAv(v: [domain(1,int(64),one)] real(64), tmp: [domain(1,int(64),one)] real(64), AtAv: [domain(1,int(64),one)] real(64))
9-
spectralnorm2.chpl:43: In function 'multiplyAtv':
10-
spectralnorm2.chpl:44: warning: inferring a default intent to be 'ref' is deprecated - please add an explicit 'ref' forall intent for 'Atv'
11-
spectralnorm2.chpl:29: called as multiplyAtv(v: [domain(1,int(64),one)] real(64), Atv: [domain(1,int(64),one)] real(64)) from function 'multiplyAtAv'
12-
spectralnorm2.chpl:17: called as multiplyAtAv(v: [domain(1,int(64),one)] real(64), tmp: [domain(1,int(64),one)] real(64), AtAv: [domain(1,int(64),one)] real(64))
135
1.274224116

‎test/studies/shootout/submitted/spectralnorm2.perfkeys

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
real
2-
verify:13: 1.274224153
2+
verify:5: 1.274224153

‎test/deprecated/ref-maybe-const-forall-intent.chpl ‎test/unstable/ref-maybe-const-forall-intent.chpl

File renamed without changes.

‎test/unstable/ref-maybe-const-forall-intent.good

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
ref-maybe-const-forall-intent.chpl:31: warning: inferring a 'ref' intent on an array in a forall is unstable - in the future this may require an explicit 'ref' forall intent for 'B'
2+
ref-maybe-const-forall-intent.chpl:70: warning: inferring a 'ref' intent on an array in a forall is unstable - in the future this may require an explicit 'ref' forall intent for 'A'
3+
ref-maybe-const-forall-intent.chpl:2: warning: inferring a 'ref' intent on an array in a forall is unstable - in the future this may require an explicit 'ref' forall intent for 'myArray1'
4+
ref-maybe-const-forall-intent.chpl:31: warning: inferring a 'ref' intent on an array in a forall is unstable - in the future this may require an explicit 'ref' forall intent for 'B'
5+
ref-maybe-const-forall-intent.chpl:50: warning: inferring a 'ref' intent on an array in a forall is unstable - in the future this may require an explicit 'ref' forall intent for 'myArrayD'
6+
0,0,0,0,0,0,0,0,0,0,

0 commit comments

Comments
 (0)