chapel-lang/chapel — pull request #411 by vasslitvinov (2014-09-13 06:03:50+00:00)
Title: Disable certain lvalue errors

MOTIVATION

The following code:

iter iRef() ref {...}
iRef() = 2;

apparently is well-defined and means
"assign 2 to each thing yielded by iRef".

Analogously, the following code should generate an "lvalue"
error because things yielded by a non-ref iterator should not
be assignable:

// from test/functions/deitz/iterators/test_var_iterator5_error.chpl
iter iVal() {...}
iVal() = 2;

With my PR #324, the code generates two errors - one pointing
to the user code, i.e. the line "iVal() = 2", the other pointing
into our internal modules.

This second error is (a) unnecessary, (b) confusing
"hey your internal modules have an error", and (c)
has a floating line number (Elliot updated .good
three times in the last couple of days).

This commit suppresses this second error.

DETAILS

The AST generated for both snippets above is the same.
Here is AST for iVal(), with embelishments:

def call_tmp:_ir_iVal
move( call_tmp call( fn iVal ) )
call( fn = call_tmp 2 ) <-------------------- [1]

where fn = is this:

// ChapelArray.chpl
proc =(ref ic: _iteratorRecord, x: iteratorIndexType(ic)) {
for e in ic do
e = x;
}

with this, slightly edited, AST:

fn =:void
def arg ic:_ref(_ir_iVal) <-- intent: ref
def arg x:int(64)
{
def _iterator:_ic_iVal
move( _iterator call( fn _getIterator deref( arg ic ) ) )
def _indexOfInterest:int(64)
move( _indexOfInterest call( fn iteratorIndex _iterator ) )
{
def e:int(64)
move( e _indexOfInterest )
call( fn = e arg x ) <-------------------- [2]
for loop( _indexOfInterest _iterator )
}
...
}

where fn = is our standard integer assignment (ref a: int, b: int).

[1]: lvalueCheck() in resolution marks this call an error
because 'call_tmp' has the flag "expr temp". I don't know
the meaning of "expr temp" and how it got there. Evidently
it works for our constness checking.

Aside: In both iVal and iRef cases, 'call_tmp' has also the flags
"temp" "maybe param" "maybe type". In iRef case, there is no "expr temp".

[2]: lvalueCheck() flags this an error because 'e' has the flag "const".

Aside: 'e' also has the flags "insert auto destroy" "index var"
in both cases.
In the iRef case, the type of 'e' and _indexOfInterest is _ref(int(64)),
happily reflecting ref-ness of the underlying iterator.

The error in [2] makes a good sense to me and [1] doesn't.
However, [1] has the right line number, and [2] points into
ChapelArray.chpl, which may confuse for the user ("this is not my error").

After consulting with David and Tom, I feel that we do not need
to report the error in [2] because the one in [1] will have been
reported already.

So how to suppress the error in [2]? I found it the simplest
to mark [2]'s enclosing function with a new flag and have
lvalueCheck() not report errors in functions with this flag.
Alternative suggestions are welcome.

Aside: I thought that when an error is in an internal module,
we climb up the call chain until we hit user code. However
I did not see code to do that. Maybe we only do this for warnings?