chapel-lang/chapel — pull request #90 by vasslitvinov (2014-07-31 20:20:09+00:00)
Title: Merging ChapelRangeBase.chpl into ChapelRange.chpl

Merged ChapelRangeBase.chpl into ChapelRange.chpl.

ChapelRangeBase.chpl was split out from ChapelRange.chpl (SVN r18549)
when ambiguous alignment was introduced, via range._aligned.

As part of the split, the fields _low, _high, _stride, _alignment,
plus a lot of the code to handle them, were moved to the record 'rangeBase'
and to the file ChapelRangeBase.chpl.
The record 'range', instead of these four fields directly, got a single
field _base that was a rangeBase.

Given our experiences with this arrangement since then, we would like
to revert it. We expect that having _base's fields directly in 'range'
would simplify the generated code. Also for a given functionality on
'range', we expect it to be easier to examine/maintain when it is
in a single function instead of being split into two.

IMPLEMENTATION STRATEGY

Primarily I inlined by hand each 'rangeBase' function into the
corresponding 'range' function. Some helper functions moved
from ChapelRangeBase.chpl to ChapelRange.chpl verbatim.

While there, I simplified some things where it looked easy enough.

DETAILED NOTES

* Simplified interior(), exterior() by merging the stridable and
unstridable cases.

For that, I introduced _effAlmt(), which is admittedly not very elegant.

* Fixed a bug in exterior().

* In general, paren-less member functions cannot be called without
specifying the receiver, i.e. cannot omit the implicit "this.".
So I made my new function paren-ful:
_effAlmt

and added explicit "this." for the following existing functions:
first
length

* Simplified this function:

proc rangeBase.translate(i: integral)
{
if _isSignedType(i.type) then
return this + i:idxType;
else if i >= 0 then return this + i:idxType;
else return this - abs(i):idxType;
}

down to "return this + i:idxType;". Which seems equivalent.

* I merged the two separate ==(rangeBase*2) functions
(one was for bounded ranges and the other was "else")
into a single ==(range*2), separating them by "if isBoundedRange(r1)".

* I removed the range constructor that took rangeBase as argument,
and a couple of other rangeBase functions that are now unused.

* In former chpl__count(), now chpl_count_help(),
when the degenerate range is returned,
I changed _alignment from 1 to 0 and _aligned from r._aligned with false.

As a result, a couple of empty ranges now print without "align 1" in:

types/range/hilde/count.chpl

* Some library and test code references range fields directly.
I removed _base there:

modules/dists/BlockDist.chpl
modules/dists/dims/BlockCycDim.chpl
test/arrays/vass/slices-with-negative-strides-1.chpl
test/release/examples/benchmarks/miniMD/helpers/StencilDist.chpl

* I removed this test (approved by Sung):

types/records/sungeun/chapelTypes/record_rangeBase.chpl

It would break because it relies on 'rangeBase' from the modules,
which is no longer there.

A similiar test that uses 'range' instead of 'rangeBase',
still exists and passes:

types/records/sungeun/chapelTypes/record_range.chpl

* ChapelRangeBase is no longer mentioned in:

test/modules/sungeun/init/printModuleInitOrder.good