Chapel: Directly inlined iterators modify local tmp instead of arg

Created on 7 Sep 2017  路  6Comments  路  Source: chapel-lang/chapel

Summary of Problem

Updates to arguments passed to an iterator aren't preserved by direct iterator
inlining (--inline-iterators). e.g.

iter myiter(in arg) {
  for i in 1..10 {
    arg += 1;
    yield arg;
  }
}

for i in myiter(1) do writeln(i);

will always yield 1 instead of 1, 2, 3, ...

Note that this is a direct iterator inlining only bug. The test works if
inlining is done using the zippered iterator inlining code
(--no-inline-iterators --optimize-loop-iterators)

I _think_ this is likely a bug in replaceIteratorFormalsWithIteratorFields(),
but I'm not positive.

Associated Future Test(s):
test/functions/iterators/elliot/yieldArgIter/yieldArgIter.chpl #7232

Compiler Bug

All 6 comments

I have a branch that attempts to inline all non-zippered iterators, not just those with only 1 yield, and ~12 tests break because of this bug.

Related to https://github.com/chapel-lang/chapel/issues/6757

@daviditen Do you have any time to look at this?

I looked at older versions to see if this has ever worked and 1.14.0 works. I did a binary search to see when it broke and PR #5624 on March 16 is when it stopped working. With that hint I'll continue looking.

This change to functionResolution.cpp:5585 is where this test goes wrong. Changing that line back makes the test pass.

The commit message for that commit states that the removed condition was apparently the reverse of what it should be. I think this is correct, and the reverse should be added back in, along with another case for fn->hasFlag(FLAG_ITERATOR_FN) since we typically try to inline iterators.

I have this change in testing right now.

[Test Summary - 170908.165338]
[Summary: #Successes = 7761 | #Failures = 0 | #Futures = 0]

Closed by #7253

Was this page helpful?
0 / 5 - 0 ratings