Skip to content

Commit

Permalink
Add chpl__defaultHash for borrowed object? and adjust class tests…
Browse files Browse the repository at this point in the history
… for set (#15592)

Add `chpl__defaultHash` for `borrowed object?` and adjust class tests
for set (#15592)

This PR adds an overload of `chpl__defaultHash` for
`borrowed object?`. The overload of `chpl__defaultHash` for
`borrowed object` has been removed, because all non-nilable borrowed
objects will coerce to nilable borrowed object. Futures for sets of
classes that pass after adding the overload have been migrated to
normal tests.

Prior to this PR sets of nilable classes (associative domains of
nilable classes, in truth) failed to instantiate because they were
unable to resolve a call to `chpl__defaultHash`. The compiler can
automatically coerce class type arguments with other management
styles to `borrowed` when resolving a function call. However, the
compiler cannot coerce a nilable class to a non-nilable class, which
is why resolution of `chpl__defaultHash` failed when the set
element type was `borrowed C?`.

---

Testing:

- [x] ALL on linux64 when COMM=none
- [x] ALL on linux64 when COMM=gasnet

---

Reviewed by @mppf.
  • Loading branch information
dlongnecke-cray authored May 12, 2020
1 parent f00a357 commit 8260f89
Show file tree
Hide file tree
Showing 18 changed files with 57 additions and 61 deletions.
7 changes: 4 additions & 3 deletions modules/internal/DefaultAssociative.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ module DefaultAssociative {
proc dsiMember(idx: idxType): bool {
return _findFilledSlot(idx)(0);
}

override proc dsiAdd(idx) {
// add helpers will return a tuple like (slotNum, numIndicesAdded);

Expand Down Expand Up @@ -995,8 +995,9 @@ module DefaultAssociative {
}
return hash;
}

inline proc chpl__defaultHash(o: borrowed object): uint {

// Nilable and non-nilable classes will coerce to this.
inline proc chpl__defaultHash(o: borrowed object?): uint {
return _gen_key(__primitive( "object2int", o));
}

Expand Down
57 changes: 29 additions & 28 deletions modules/standard/Set.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,26 @@ module Set {
}
}

pragma "no doc"
proc _checkElementType(type t) {
// Non-nilable shared classes fail with a strange compiler bug.
if isOwnedClass(t) || (isNonNilableClass(t) && isSharedClass(t)) then
compilerError('Sets do not support this class type', 2);

// Only error if a tuple element is a non-nilable class.
if isTuple(t) then
for param i in 0..<t.size do
if isNonNilableClass(t[i]) then
compilerError('Sets do not support tuples containing ' +
'non-nilable classes', 2);

// In the future we might support it if the set is not default-inited.
if isGenericType(t) {
compilerWarning('creating a set with element type ' + t:string, 2);
compilerError('set element type cannot currently be generic', 2);
}
}

/*
A set is a collection of unique elements. Attempting to add a duplicate
element to a set has no effect.
Expand Down Expand Up @@ -126,19 +146,8 @@ module Set {
:arg parSafe: If `true`, this set will use parallel safe operations.
*/
proc init(type eltType, param parSafe=false) {
// Only non-nilable borrowed classes work so far
if isClass(eltType) {
if !(isBorrowedClass(eltType) && isNonNilableClass(eltType)) then
compilerError('Sets do not support class types');
}
if isTuple(eltType) then
compilerError('Sets do not support tuple types');
if isGenericType(eltType) {
compilerWarning("creating a set with element type " +
eltType:string);
compilerError("set element type cannot currently be generic");
// In the future we might support it if the set is not default-inited
}
_checkElementType(eltType);

this.eltType = eltType;
this.parSafe = parSafe;
}
Expand All @@ -153,17 +162,9 @@ module Set {
:arg parSafe: If `true`, this set will use parallel safe operations.
*/
proc init(type eltType, iterable, param parSafe=false)
where canResolveMethod(iterable, "these") {
if isClass(eltType) then
compilerError('Sets do not support class types');
if isTuple(eltType) then
compilerError('Sets do not support tuple types');
if isGenericType(eltType) {
compilerWarning("creating a set with element type " +
eltType:string);
compilerError("set element type cannot currently be generic");
// In the future we might support it if the set is not default-inited
}
where canResolveMethod(iterable, "these") lifetime this < iterable {
_checkElementType(eltType);

this.eltType = eltType;
this.parSafe = parSafe;
this.complete();
Expand All @@ -179,7 +180,7 @@ module Set {
:arg other: A set to initialize this set with.
*/
proc init=(const ref other: set(?t, ?)) {
proc init=(const ref other: set(?t, ?)) lifetime this < other {
this.eltType = t;
this.parSafe = other.parSafe;
this.complete();
Expand Down Expand Up @@ -217,7 +218,7 @@ module Set {
:arg x: The element to add to this set.
*/
proc add(in x: eltType) {
proc ref add(in x: eltType) lifetime this < x {
on this {
_enter();
_dom.add(x);
Expand Down Expand Up @@ -308,7 +309,7 @@ module Set {
:return: Whether or not an element equal to `x` was removed.
:rtype: `bool`
*/
proc remove(const ref x: eltType): bool {
proc ref remove(const ref x: eltType): bool {
var result = false;

on this {
Expand All @@ -333,7 +334,7 @@ module Set {
Clearing the contents of this set will invalidate all existing
references to the elements contained in this set.
*/
proc clear() {
proc ref clear() {
on this {
_enter();
_dom.clear();
Expand Down
19 changes: 17 additions & 2 deletions test/library/standard/Set/types/SetTest.chpl
Original file line number Diff line number Diff line change
@@ -1,10 +1,25 @@
import Set.set;

proc testSet(type t) where isTuple(t) {
var s = new set(t);

var x = (new t[0](1), new t[1](2));

s.add(x);
assert(s.size == 1);

s.remove(x);
assert(s.size == 0);

if isUnmanagedClass(t) {
delete x;
}
}

proc testSet(type t) {
var s = new set(t);

var x = if isTuple(t) then (new t[1](1), new t[2](2))
else new t(1);
var x = new t(1);

s.add(x);
assert(s.size == 1);
Expand Down
3 changes: 0 additions & 3 deletions test/library/standard/Set/types/testBorrowed.bad

This file was deleted.

1 change: 0 additions & 1 deletion test/library/standard/Set/types/testNilableBorrowed.bad

This file was deleted.

1 change: 0 additions & 1 deletion test/library/standard/Set/types/testNilableBorrowed.future

This file was deleted.

4 changes: 2 additions & 2 deletions test/library/standard/Set/types/testNilableOwned.bad
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
./SetTest.chpl:3: In function 'testSet':
./SetTest.chpl:4: error: Sets do not support class types
./SetTest.chpl:19: In function 'testSet':
./SetTest.chpl:20: error: Sets do not support this class type
testNilableOwned.chpl:9: Function 'testSet' instantiated as: testSet(type t = owned T?)
3 changes: 0 additions & 3 deletions test/library/standard/Set/types/testNilableShared.bad

This file was deleted.

1 change: 0 additions & 1 deletion test/library/standard/Set/types/testNilableShared.future

This file was deleted.

3 changes: 0 additions & 3 deletions test/library/standard/Set/types/testNilableTuple.bad

This file was deleted.

1 change: 0 additions & 1 deletion test/library/standard/Set/types/testNilableTuple.future

This file was deleted.

3 changes: 0 additions & 3 deletions test/library/standard/Set/types/testNilableUnmanaged.bad

This file was deleted.

This file was deleted.

4 changes: 2 additions & 2 deletions test/library/standard/Set/types/testOwned.bad
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
./SetTest.chpl:3: In function 'testSet':
./SetTest.chpl:4: error: Sets do not support class types
./SetTest.chpl:19: In function 'testSet':
./SetTest.chpl:20: error: Sets do not support this class type
testOwned.chpl:9: Function 'testSet' instantiated as: testSet(type t = owned T)
4 changes: 2 additions & 2 deletions test/library/standard/Set/types/testShared.bad
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
./SetTest.chpl:3: In function 'testSet':
./SetTest.chpl:4: error: Sets do not support class types
./SetTest.chpl:19: In function 'testSet':
./SetTest.chpl:20: error: Sets do not support this class type
testShared.chpl:9: Function 'testSet' instantiated as: testSet(type t = shared T)
2 changes: 1 addition & 1 deletion test/library/standard/Set/types/testTuple.bad
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
./SetTest.chpl:3: In function 'testSet':
./SetTest.chpl:4: error: Sets do not support tuple types
./SetTest.chpl:4: error: Sets do not support tuples containing non-nilable classes
testTuple.chpl:9: Function 'testSet' instantiated as: testSet(type t = 2*shared T)
3 changes: 0 additions & 3 deletions test/library/standard/Set/types/testUnmanaged.bad

This file was deleted.

1 change: 0 additions & 1 deletion test/library/standard/Set/types/testUnmanaged.future

This file was deleted.

0 comments on commit 8260f89

Please sign in to comment.