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

The name is too short to be unambiguous, so either you need to make the name longer to make it self-documenting, or you need to add a comment to tell other programmers what are the intended semantics of the function.
________________________________
From: Vassily Litvinov [notifications@github.com]
Sent: Thursday, July 31, 2014 2:49 PM
To: chapel-lang/chapel
Cc: Tom Hildebrandt
Subject: Re: [chapel] Merging ChapelRangeBase.chpl into ChapelRange.chpl (#90)


In modules/internal/ChapelRange.chpl:

>    inline proc range.translate(i)
>    {
>      compilerError("offsets must be of integral type");
>    }
> +
> +  // private, for use in interior/exterior
> +  inline proc range._effAlmt()       where stridable return _alignment;


If we make the name longer, the 6 lines that are like following will overflow, which will be less pretty:

  return new range(idxType, boundedType, stridable,
                   _low, _low - 1 - i, stride, _effAlmt(), _aligned);


I favor the shorter presentation.

My reason to keep it private is so we do not need to explain what it does. Also, we have not had any requests from users for this functionality.

—
Reply to this email directly or view it on GitHub<https://github.com/chapel-lang/chapel/pull/90/files#r15672304>.