Skip to content

Commit

Permalink
Revert "[External] [stdlib] Support Dict.popitem() (#40688)"
Browse files Browse the repository at this point in the history
This PR introduces nondeterminism into the testsuite. `test_dict.mojo`
nondeterministically fails with

```
[M] ➜  modular git:(1853f9d3e9) mojo /Users/jeff/Documents/modular/******/test/stdlib/collections/test_dict.mojo
Test test_basic ...PASS
Test test_multiple_resizes ...PASS
Test test_big_dict ...PASS
Test test_compact ...PASS
Test test_compact_with_elements ...PASS
Test test_pop_default ...PASS
Test test_key_error ...PASS
Test test_iter ...PASS
Test test_iter_keys ...PASS
Test test_iter_values ...PASS
Test test_iter_values_mut ...PASS
Test test_iter_items ...PASS
Test test_dict_copy ...PASS
Test test_dict_copy_add_new_item ...PASS
Test test_dict_copy_delete_original ...PASS
Test test_dict_copy_calls_copy_constructor ...PASS
Test test_dict_update_nominal ...PASS
Test test_dict_update_empty_origin ...PASS
Test test_dict_update_empty_new ...PASS
Test test_mojo_issue_1729 ...PASS
Test test dict or ...PASS
Test test dict popteim ...get: wrong variant type
Please submit a bug report to https://github.com/modularml/mojo/issues and include the crash backtrace along with all the relevant source codes.
Stack dump:
0.      Program arguments: mojo /Users/jeff/Documents/modular/******/test/stdlib/collections/test_dict.mojo
 #0 0x00000001043a10b0 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/Users/jeff/Documents/modular/.derived/build-relwithdebinfo/bin/mojo+0x1000c90b0)
 #1 0x000000010439f210 llvm::sys::RunSignalHandlers() (/Users/jeff/Documents/modular/.derived/build-relwithdebinfo/bin/mojo+0x1000c7210)
 #2 0x00000001043a1750 SignalHandler(int) (/Users/jeff/Documents/modular/.derived/build-relwithdebinfo/bin/mojo+0x1000c9750)
 #3 0x00000001ab1b2a24 (/usr/lib/system/libsystem_platform.dylib+0x18042ea24)
 #4 0xffff8002a81b8510
 #5 0x00000001047c1608 M::KGEN::ExecutionEngine::runProgram(llvm::StringRef, llvm::StringRef, llvm::function_ref<M::ErrorOrSuccess (void*)>) (/Users/jeff/Documents/modular/.derived/build-relwithdebinfo/bin/mojo+0x1004e9608)
 #6 0x00000001042f8270 executeMain(mlir::ModuleOp, mlir::SymbolTable const&, M::KGEN::ExecutionEngine*, M::LLCL::Runtime&, llvm::ArrayRef<char const*>) (/Users/jeff/Documents/modular/.derived/build-relwithdebinfo/bin/mojo+0x100020270)
 #7 0x00000001042f7cb8 run(M::State const&) (/Users/jeff/Documents/modular/.derived/build-relwithdebinfo/bin/mojo+0x10001fcb8)
 #8 0x00000001042df774 main (/Users/jeff/Documents/modular/.derived/build-relwithdebinfo/bin/mojo+0x100007774)
 #9 0x00000001aae2bf28
[1]    44318 trace trap  mojo
```

MODULAR_ORIG_COMMIT_REV_ID: ee1c665669902106df680fe6c6d2599897665ff5
  • Loading branch information
Jeff Niu authored and modularbot committed May 29, 2024
1 parent a28de9d commit b9453e9
Show file tree
Hide file tree
Showing 3 changed files with 0 additions and 48 deletions.
4 changes: 0 additions & 4 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ what we publish.

### ⭐️ New

- `Dict` now supports `popitem`, which removes and returns the last item in the `Dict`.
([PR #2701](https://github.com/modularml/mojo/pull/2701)
by [@jayzhan211](https://github.com/jayzhan211))

- Add a `sort` function for list of `ComparableCollectionElement`s.
[PR #2609](https://github.com/modularml/mojo/pull/2609) by
[@mzaks](https://github.com/mzaks)
Expand Down
28 changes: 0 additions & 28 deletions stdlib/src/collections/dict.mojo
Original file line number Diff line number Diff line change
Expand Up @@ -799,34 +799,6 @@ struct Dict[K: KeyElement, V: CollectionElement](
return default.value()[]
raise "KeyError"

fn popitem(inout self) raises -> DictEntry[K, V]:
"""Remove and return a (key, value) pair from the dictionary. Pairs are returned in LIFO order.
popitem() is useful to destructively iterate over a dictionary, as often used in set algorithms.
If the dictionary is empty, calling popitem() raises a KeyError.
Args: None
Returns:
Last dictionary item
Raises:
"KeyError" if the dictionary is empty.
"""

var key = Optional[K](None)
var val = Optional[V](None)

for item in reversed(self.items()):
key = Optional(item[].key)
val = Optional(item[].value)
break

if key:
_ = self.pop(key.value()[])
return DictEntry[K, V](key.value()[], val.value()[])

raise "KeyError: popitem(): dictionary is empty"

fn keys(
self: Reference[Self, _, _]
) -> _DictKeyIter[K, V, self.is_mutable, self.lifetime]:
Expand Down
16 changes: 0 additions & 16 deletions stdlib/test/collections/test_dict.mojo
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,6 @@ def test_dict():
test["test_dict_update_empty_new", test_dict_update_empty_new]()
test["test_mojo_issue_1729", test_mojo_issue_1729]()
test["test dict or", test_dict_or]()
test["test dict popteim", test_dict_popitem]()


def test_taking_owned_kwargs_dict(owned kwargs: OwnedKwargsDict[Int]):
Expand Down Expand Up @@ -513,21 +512,6 @@ def test_find_get():
assert_equal(some_dict.get("not_key", 0), 0)


def test_dict_popitem():
var dict = Dict[String, Int]()
dict["a"] = 1
dict["b"] = 2

var item = dict.popitem()
assert_equal(item.key, "b")
assert_equal(item.value, 2)
item = dict.popitem()
assert_equal(item.key, "a")
assert_equal(item.value, 1)
with assert_raises(contains="KeyError"):
_ = dict.popitem()


fn test_clear() raises:
var some_dict = Dict[String, Int]()
some_dict["key"] = 1
Expand Down

0 comments on commit b9453e9

Please sign in to comment.