Skip to content

Commit

Permalink
Enable sorting for Sets
Browse files Browse the repository at this point in the history
Summary: Now that Sets retain insertion order it would be handy if Sets could be
sorted using PHP's sort builtin functions. This diff leverages to shared
implementation between Map and Set to enable sorting for Sets.

Reviewed By: ptc, eletuchy

Differential Revision: D1435214
  • Loading branch information
paroski authored and facebook-github-bot committed Jul 15, 2014
1 parent ea92f3b commit 89ec52e
Show file tree
Hide file tree
Showing 3 changed files with 157 additions and 18 deletions.
44 changes: 26 additions & 18 deletions hphp/runtime/ext/ext_array.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2132,9 +2132,8 @@ php_sort(VRefParam container, int sort_flags,
return true;
}
// other collections are not supported:
// - mapping types require associative sort
// - frozen types are not to be modified
// - set types are not ordered
// - Maps and Sets require associative sort
// - Immutable collections are not to be modified
}
throw_expected_array_or_collection_exception();
return false;
Expand All @@ -2159,9 +2158,10 @@ php_asort(VRefParam container, int sort_flags,
}
if (container.isObject()) {
ObjectData* obj = container.getObjectData();
if (obj->getCollectionType() == Collection::MapType) {
BaseMap* mp = static_cast<BaseMap*>(obj);
mp->asort(sort_flags, ascending);
if (obj->getCollectionType() == Collection::MapType ||
obj->getCollectionType() == Collection::SetType) {
HashCollection* hc = static_cast<HashCollection*>(obj);
hc->asort(sort_flags, ascending);
return true;
}
}
Expand All @@ -2179,9 +2179,10 @@ php_ksort(VRefParam container, int sort_flags, bool ascending) {
}
if (container.isObject()) {
ObjectData* obj = container.getObjectData();
if (obj->getCollectionType() == Collection::MapType) {
BaseMap* mp = static_cast<BaseMap*>(obj);
mp->ksort(sort_flags, ascending);
if (obj->getCollectionType() == Collection::MapType ||
obj->getCollectionType() == Collection::SetType) {
HashCollection* hc = static_cast<HashCollection*>(obj);
hc->ksort(sort_flags, ascending);
return true;
}
}
Expand Down Expand Up @@ -2247,9 +2248,8 @@ bool f_usort(VRefParam container, const Variant& cmp_function) {
return vec->usort(cmp_function);
}
// other collections are not supported:
// - mapping types require associative sort
// - frozen types are not to be modified
// - set types are not ordered
// - Maps and Sets require associative sort
// - Immutable collections are not to be modified
}
throw_expected_array_or_collection_exception();
return false;
Expand All @@ -2268,10 +2268,14 @@ bool f_uasort(VRefParam container, const Variant& cmp_function) {
}
if (container.isObject()) {
ObjectData* obj = container.getObjectData();
if (obj->getCollectionType() == Collection::MapType) {
BaseMap* mp = static_cast<BaseMap*>(obj);
return mp->uasort(cmp_function);
if (obj->getCollectionType() == Collection::MapType ||
obj->getCollectionType() == Collection::SetType) {
HashCollection* hc = static_cast<HashCollection*>(obj);
return hc->uasort(cmp_function);
}
// other collections are not supported:
// - Vectors require a non-associative sort
// - Immutable collections are not to be modified
}
throw_expected_array_or_collection_exception();
return false;
Expand All @@ -2285,10 +2289,14 @@ bool f_uksort(VRefParam container, const Variant& cmp_function) {
}
if (container.isObject()) {
ObjectData* obj = container.getObjectData();
if (obj->getCollectionType() == Collection::MapType) {
BaseMap* mp = static_cast<BaseMap*>(obj);
return mp->uksort(cmp_function);
if (obj->getCollectionType() == Collection::MapType ||
obj->getCollectionType() == Collection::SetType) {
HashCollection* hc = static_cast<HashCollection*>(obj);
return hc->uksort(cmp_function);
}
// other collections are not supported:
// - Vectors require a non-associative sort
// - Immutable collections are not to be modified
}
throw_expected_array_or_collection_exception();
return false;
Expand Down
27 changes: 27 additions & 0 deletions hphp/test/slow/collection_classes/set-sort.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?hh
function main() {
for ($i = 0; $i < 8; ++$i) {
if ($i < 4) {
$s = Set {3, 4, 1, 2};
} else {
$s = Set {'b', 'a', 'd', 'c'};
}
var_dump($s);
switch ($i % 4) {
case 0: asort($s); break;
case 1: ksort($s); break;
case 2:
uasort($s,
function ($l, $r) { return $l < $r ? -1 : ($l === $r ? 0 : 1); });
break;
case 3:
uksort($s,
function ($l, $r) { return $l < $r ? -1 : ($l === $r ? 0 : 1); });
break;
}
var_dump($s);
unset($s);
echo "----\n";
}
}
main();
104 changes: 104 additions & 0 deletions hphp/test/slow/collection_classes/set-sort.php.expect
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
object(HH\Set)#1 (4) {
int(3)
int(4)
int(1)
int(2)
}
object(HH\Set)#1 (4) {
int(1)
int(2)
int(3)
int(4)
}
----
object(HH\Set)#1 (4) {
int(3)
int(4)
int(1)
int(2)
}
object(HH\Set)#1 (4) {
int(1)
int(2)
int(3)
int(4)
}
----
object(HH\Set)#1 (4) {
int(3)
int(4)
int(1)
int(2)
}
object(HH\Set)#1 (4) {
int(1)
int(2)
int(3)
int(4)
}
----
object(HH\Set)#1 (4) {
int(3)
int(4)
int(1)
int(2)
}
object(HH\Set)#1 (4) {
int(1)
int(2)
int(3)
int(4)
}
----
object(HH\Set)#1 (4) {
string(1) "b"
string(1) "a"
string(1) "d"
string(1) "c"
}
object(HH\Set)#1 (4) {
string(1) "a"
string(1) "b"
string(1) "c"
string(1) "d"
}
----
object(HH\Set)#1 (4) {
string(1) "b"
string(1) "a"
string(1) "d"
string(1) "c"
}
object(HH\Set)#1 (4) {
string(1) "a"
string(1) "b"
string(1) "c"
string(1) "d"
}
----
object(HH\Set)#1 (4) {
string(1) "b"
string(1) "a"
string(1) "d"
string(1) "c"
}
object(HH\Set)#1 (4) {
string(1) "a"
string(1) "b"
string(1) "c"
string(1) "d"
}
----
object(HH\Set)#1 (4) {
string(1) "b"
string(1) "a"
string(1) "d"
string(1) "c"
}
object(HH\Set)#1 (4) {
string(1) "a"
string(1) "b"
string(1) "c"
string(1) "d"
}
----

0 comments on commit 89ec52e

Please sign in to comment.