chapel-lang/chapel — pull request #157 by vasslitvinov (2014-08-14 22:40:19+00:00)
Title: Improved constness checking on fields; adjusted Chapel code.

COMPILER CHANGES

functionResolution.cpp
wrappers.cpp

* Improved the logic of const checking added in SVN r22963.

** Check field accesses on result of *any* function with FLAG_REF_TO_CONST,
not just field accessors.

** Check field accesses also to const formals, e.g. blank-intent records.

For that, I replaced the check: baseSym->hasFlag(FLAG_CONST)
with: baseSym->isConstant()

** Propagate FLAG_FIELD_ACCESSOR and FLAG_REF_TO_CONST
to wrappers.

* Factored out some const-checking code in resolveNormalCall()
into a separate setFlagsForConstAccess().

* For now, promotion wrappers are not checked, due to scheduling concerns.
There is a corresponding 'if' condition in setFlagsForConstAccess().
This might produce incorrect errors in some cases, not exercised
in our current code base.

MODULE CHANGES

* Switched the field locale.numCores to be 'var' (from 'const').
That's because it is modified in RootLocale.init(), which
is invoked outside of a constructor - in chpl_init_rootLocale().
This was Sung's suggestion, making us forward-looking to having
a node's core count adjustable at run time.

modules/internal/ChapelLocale.chpl

* range.member no longer modifies its arguments. For that, I added a
helper function that takes one of the arguments by 'in' intent, to
indicate an explicit copy.

I did not make the arguments of range.member themselves by 'in' intent,
assuming that in many cases they will be passed by reference (blank
intent), so copies will not be made.

modules/internal/ChapelRange.chpl

* Changed several formals of the type 'qio_regexp_t'
from 'ref' to 'const ref':

modules/standard/Regexp.chpl
qio_regexp_retain
qio_regexp_get_options
qio_regexp_get_pattern
qio_regexp_get_ncaptures
qio_regexp_ok
qio_regexp_error

modules/standard/IO.chpl
qio_regexp_channel_match

Correspondingly, in the runtime C code, changed
qio_regexp_t* regexp --> const qio_regexp_t* regexp

runtime/include/qio/qio_regexp.h
runtime/src/qio/regexp/re2/re2-interface.cc
runtime/src/qio/regexp/none/fail-regexp.c

To motivate, consider for example proc =(ref ret:regexp, x:regexp)

- It takes 'x' by blank intent. 'regexp' being a record,
this means that 'x' is constant and it is illegal
to modify its fields.

- It passes x._regexp to qio_regexp_retain() by 'ref',
which is considered to be a modification, and so is illegal.

If we want at all to allow assignment from a "const" regexp,
we want the above process to be read-only for 'x'. Therefore
we want qio_regexp_retain() to be read-only as well.

Another solution is to make a copy on the fly and modify that one.
However, given that the modification is related to reference counting,
that would not make sense.

I consulted with Michael on the above and he expressed a preference
not to bake constness into the interface. However he has not,
as of this writing, offered an alternative solution.

* Replaced the blank-intent 'style' formal with 'in local_style',
to indicate an explicit copy for local modifications, in:

modules/standard/IO.chpl
channel.channel()
file.lines()

Correspondingly, removed one 'local_style' variable.

TEST CHANGES

* range.member no longer modifies its argument

test/arrays/vass/slices-with-negative-strides-1.good

* specify 'ref' task intent so the variable can be modified in a 'begin'

test/parallel/begin/waynew/record1.chpl
test/parallel/begin/waynew/record2.chpl

* pass a record by 'in' to allow local modifications;
it looks like the actual better remains unmodified

test/studies/hpcc/HPL/marybeth/HPL.chpl

* blank-intent record formals no longer allow field modification,
so this future is removed

test/types/records/const-checking/todo-assign-to-record-formal-error.future
test/types/records/const-checking/todo-assign-to-record-formal-error.bad