Add chpl__defaultHash for borrowed object?and adjust class tests for set
#15592
Conversation
|
@mppf Would you be willing to review this? A quick update on what is left:
|
| } | ||
|
|
||
| // Nilable classes will coerce to this. | ||
| inline proc chpl__defaultHash(o: borrowed object?): uint { |
There was a problem hiding this comment.
Actually you should be able to just have this version and leave out the above since any class should coerce to borrowed object?. Could you give that a try?
There was a problem hiding this comment.
I should have thought about that. That should work nicely.
| 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. |
There was a problem hiding this comment.
Yeah, but if it were default-initialized, the compiler should figure out the concrete type.
modules/standard/Set.chpl
Outdated
| @@ -441,7 +445,7 @@ module Set { | |||
| :return: An array containing a copy of each of the elements in this set. | |||
| :rtype: `[] eltType` | |||
| */ | |||
| proc const toArray(): [] eltType { | |||
| proc const toArray(): [] eltType where isCopyableType(eltType) { | |||
There was a problem hiding this comment.
For functions like this, I generally prefer the approach of a param conditional guarding a compilation error. The error can be more friendly than the resolution error. But there might be a reason the resolution error is nicer here?
There was a problem hiding this comment.
I'm actually going to remove the where clauses from this PR and come back to them in another effort, because I just discovered the |= operators aren't working as expected 🤭
This PR adds an overload of
chpl__defaultHashforborrowed object?. The overload ofchpl__defaultHashfor non-nilable objects has been removed, because non-nilable classes will coerce to nilable 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 failed to instantiate because they were unable to resolve a call to
chpl__defaultHash. The compiler is able to automatically coerce arguments of typeowned,shared, andunmanagedtoborrowedwhen resolving a function call. However we did not offer an overload ofchpl__defaultHashthat accepted a nilable borrowed, causing resolution of thesetinitializer to fail when the element type was a nilable class.Testing:
ALLonlinux64whenCHPL_COMM=noneALLonlinux64whenCHPL_COMM=gasnet