Change set.pop to return the last element

Summary: [It was decided](https://fb.workplace.com/groups/starlark/posts/1505528543469302).

Reviewed By: perehonchuk

Differential Revision: D64098426

fbshipit-source-id: f097c8d4677a06ab5374b673b126102d2dc42782

starlark/src/tests/go.rs

@@ -105,7 +105,6 @@ fn test_go() {
                 "cannot insert into frozen hash table", // We don't actually have freeze
                 "cannot clear frozen hash table",
                 "discard: cannot delete from frozen hash table",
-                "pop: cannot delete from frozen hash table",
             ],
         ),
         &[],

starlark/src/values/types/set/methods.rs

@@ -271,27 +271,27 @@ pub(crate) fn set_methods(builder: &mut MethodsBuilder) {
         Ok(NoneType)
     }
 
-    /// Removes and returns the first element of a set.
+    /// Removes and returns the **last** element of a set.
     ///
-    /// `S.pop()` removes and returns the first element of the set S.
+    /// `S.pop()` removes and returns the last element of the set S.
     ///
     /// `pop` fails if the set is empty, or if the set is frozen or has active iterators.
-    /// Time complexity of this operation is *O(N)* where *N* is the number of entries in the set.
+    /// Time complexity of this operation is *O(1)*.
     ///
     /// ```
     /// # starlark::assert::is_true(r#"
     /// x = set([1, 2, 3])
     /// # (
-    /// x.pop() == 1
+    /// x.pop() == 3
     /// # and
     /// x.pop() == 2
     /// # and
-    /// x == set([3])
+    /// x == set([1])
     /// # )"#);
     /// ```
     fn pop<'v>(this: Value<'v>) -> starlark::Result<Value<'v>> {
         let mut set = SetMut::from_value(this)?;
-        match set.aref.content.shift_remove_index(0) {
+        match set.aref.content.pop() {
             Some(x) => Ok(x),
             None => Err(value_error!("pop from an empty set")),
         }
@@ -561,7 +561,7 @@ mod tests {
 
     #[test]
     fn test_pop() {
-        assert::is_true("x = set([1, 0]); (x.pop() == 1 and x.pop() == 0 and x == set())");
+        assert::is_true("x = set([1, 0]); (x.pop() == 0 and x.pop() == 1 and x == set())");
     }
 
     #[test]

starlark/testcases/eval/go/set.star

@@ -147,14 +147,14 @@ asserts.fails(lambda: discard_set.discard(1), "discard: cannot delete from froze
 
 # pop
 pop_set = set([1,2,3])
-asserts.eq(pop_set.pop(), 1)
-asserts.eq(pop_set.pop(), 2)
-asserts.eq(pop_set.pop(), 3)
-asserts.fails(lambda: pop_set.pop(), "pop: empty set")
-pop_set.add(1)
-pop_set.add(2)
-freeze(pop_set)
-asserts.fails(lambda: pop_set.pop(), "pop: cannot delete from frozen hash table")
+# asserts.eq(pop_set.pop(), 1)
+# asserts.eq(pop_set.pop(), 2)
+# asserts.eq(pop_set.pop(), 3)
+# asserts.fails(lambda: pop_set.pop(), "pop: empty set")
+# pop_set.add(1)
+# pop_set.add(2)
+# freeze(pop_set)
+# asserts.fails(lambda: pop_set.pop(), "pop: cannot delete from frozen hash table")
 
 # clear
 clear_set = set([1,2,3])