Give _defaultOf() precedence over default constructor calls

This is a post-release cleanup that resolves the competition between _defaultOf() and user default constructor calls.

When the _defaultOf() feature was added, it provided type-specific default values for all types but did not call a user-defined default constructor if it was available.  (Instead, it called the compiler-supplied all-fields constructor -- behavior that was consistent with the existing implementation at that time.)  When I added user-default constructor calls, I did so in a way that would prefer the user-default constructor if it was present and otherwise use defaultOf().

That's not a particularly good design, because whether _defaultOf() is called for a particular type depends on whether a user-defined default constructor is present for that type.  If the user was also overriding _defaultOf(), he would have to read the spec to discover why his routine was not being called.  It is more consistent to give either _defaultOf() or the default constructor absolute priority.  If priority were given to default constructor calls, then the need for _defaultOf() would vanish.  But doing that depends on being able to initialize a variable of fundamental type by calling a constructor on it.  This is currently unsupported.

That makes _defaultOf() the obvious choice.  It provides a shim to support default initialization for types that do not yet expose constructors, and with this change, its default behavior for records is consistent with calling the zero-args constructor directly.

###THE CHANGE
In the place where resolution used to call the user-default constructor if it existed and _defaultOf() otherwise, _defaultOf() is now called unconditionally.  In the place where the compiler-supplied implementation of _defaultOf() for records used to call the compiler-generated all-fields constructor, it now calls the zero-args constructor by name.  Removing the special case for user-defined default constructor calls from resolution allowed a considerable amount of redundant code to be removed.

On Oct 27, 2014, at 2:28 PM, Lydia Duncan <notifications@github.com<mailto:notifications@github.com>> wrote:


In test/types/records/sungeun/chapelTypes/record_range.chpl:

> @@ -1,3 +1,17 @@
> +// Make sure that a locally-defined range type overrides the
> +// version implicitly used from ChapelRange.
> +//
> +// Quoted from the 0.96 version of the spec:
> +//
> +// "Symbols made available by a use statement are at an outer scope from those
> +//  defined directly in the scope where the use statement occurs, but at a
> +//  nearer scope than symbols defined in the scope containing the scope where
> +//  the use statement occurs.
> +//
> +// "If used modules themselves use other modules, symbols are scoped according
> +//  the depth of use statements followed to find them. It is an error for two
> +//  variables, types, or modules to be defined at the same depth."
> +//


I think if we don't have a future related to this, we are likely to never return to this test and see this text, since it will pass as a result of your change in the ChapelRange module. Either this test should be turned into a future or a new test should be made to demonstrate the failure to put precedence on standard module names.

Personally, I think it is a user error for "range" to be used as a record name, since it is otherwise considered a keyword. I'd like to see this be a parsing error. Maybe there should be a discussion about how to handle this case?



Chapel has a reasonably modest list of keywords; 60 currently and one more held reserve.
“range” is not one of them.  Nor is “int” (for example).

We can imagine that there are pros/cons to this approach.


Mike







—
Reply to this email directly or view it on GitHub<https://github.com/chapel-lang/chapel/pull/678/files#r19438647>.