Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Detect unhashable object types at the ASR level #2664

Merged
merged 8 commits into from
Apr 27, 2024
Merged

Detect unhashable object types at the ASR level #2664

merged 8 commits into from
Apr 27, 2024

Conversation

kmr-srbh
Copy link
Contributor

@kmr-srbh kmr-srbh commented Apr 24, 2024 •
edited
Loading

Fixes #2663

Working for dict is shown below. It is the same for set.

List

from lpython import i32

my_dict: dict[list[i32], str] = {[1, 2]: "first", [3, 4]: "second"}
(base) saurabh-kumar@Awadh:~/Projects/System/lpython$ ./src/bin/lpython ./examples/example.py
semantic error: Unhashable type: 'list'
 --> ./examples/example.py:3:15
  |
3 | my_dict: dict[list[i32], str] = {[1, 2]: "first", [3, 4]: "second"}
  |               ^^^^^^^^^ Mutable type 'list' cannot become a key in dict. Hint: Use an immutable type for key.


Note: Please report unclear or confusing messages as bugs at
https://github.com/lcompilers/lpython/issues.

Dictionary

from lpython import i32

my_dict: dict[dict[i32, str], str] = {{1: "a", 2: "b"}: "first", {3: "c", 4: "d"}: "second"}
(base) saurabh-kumar@Awadh:~/Projects/System/lpython$ ./src/bin/lpython ./examples/example.py
semantic error: Unhashable type: 'dict'
 --> ./examples/example.py:3:15
  |
3 | my_dict: dict[dict[i32, str], str] = {{1: "a", 2: "b"}: "first", {3: "c", 4: "d"}: "second"}
  |               ^^^^^^^^^^^^^^ Mutable type 'dict' cannot become a key in dict. Hint: Use an immutable type for key.


Note: Please report unclear or confusing messages as bugs at
https://github.com/lcompilers/lpython/issues.

Set

from lpython import i32

my_dict: dict[set[str], str] = {{1, 2}: "first", {3, 4}: "second"}
(base) saurabh-kumar@Awadh:~/Projects/System/lpython$ ./src/bin/lpython ./examples/example.py
semantic error: Unhashable type: 'set'
 --> ./examples/example.py:3:15
  |
3 | my_dict: dict[set[str], str] = {{1, 2}: "first", {3, 4}: "second"}
  |               ^^^^^^^^ Mutable type 'set' cannot become a key in dict. Hint: Use an immutable type for key.


Note: Please report unclear or confusing messages as bugs at
https://github.com/lcompilers/lpython/issues.

@kmr-srbh kmr-srbh changed the title Detect unhashable types for dict key at ASR level Detect unhashable object types at the ASR level Apr 25, 2024
Shaikh-Ubaid
Shaikh-Ubaid reviewed Apr 25, 2024
View reviewed changes
Copy link
Collaborator

@Shaikh-Ubaid Shaikh-Ubaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current approach in this PR seems fine to me. We should also check for mutable/unhashable types in the type declaration for a dict. We can do that later. Since this PR does not check on declaration type, but checks on the dict constant, let's update the error tests added in this PR to not use a type declaration. For example, we can have tests like:

% cat examples/expr2.py 
from lpython import i32

def f():
    print({{1, 2}: "first", {3, 4}: "second"})

f()
% lpython examples/expr2.py
Internal Compiler Error: Unhandled exception
Traceback (most recent call last):
  File "/Users/ubaid/Desktop/OpenSource/lpython/src/bin/lpython.cpp", line 1999
    err = compile_python_using_llvm(arg_file, tmp_o, runtime_library_dir,
  File "/Users/ubaid/Desktop/OpenSource/lpython/src/bin/lpython.cpp", line 878
    res = fe.get_llvm3(*asr, pass_manager, diagnostics, infile);
  File "/Users/ubaid/Desktop/OpenSource/lpython/src/lpython/python_evaluator.cpp", line 71
    run_fn, infile);
  File "/Users/ubaid/Desktop/OpenSource/lpython/src/libasr/codegen/asr_to_llvm.cpp", line 9779
    v.visit_asr((ASR::asr_t&)asr);
  File "../libasr/asr.h", line 5177
  File "../libasr/asr.h", line 5153
  File "../libasr/asr.h", line 5178
  File "../libasr/asr.h", line 4882
  File "/Users/ubaid/Desktop/OpenSource/lpython/src/libasr/codegen/asr_to_llvm.cpp", line 932
    ASR::symbol_t *mod = x.m_symtab->get_symbol(item);
  File "../libasr/asr.h", line 5180
  File "../libasr/asr.h", line 4890
  File "/Users/ubaid/Desktop/OpenSource/lpython/src/libasr/codegen/asr_to_llvm.cpp", line 3007
    finish_module_init_function_prototype(x);
  File "/Users/ubaid/Desktop/OpenSource/lpython/src/libasr/codegen/asr_to_llvm.cpp", line 3994
    ASR::Function_t *s = ASR::down_cast<ASR::Function_t>(item.second);
  File "/Users/ubaid/Desktop/OpenSource/lpython/src/libasr/codegen/asr_to_llvm.cpp", line 3750
    visit_procedures(x);
  File "/Users/ubaid/Desktop/OpenSource/lpython/src/libasr/codegen/asr_to_llvm.cpp", line 3947
    this->visit_stmt(*x.m_body[i]);
  File "../libasr/asr.h", line 5197
  File "../libasr/asr.h", line 4929
  File "/Users/ubaid/Desktop/OpenSource/lpython/src/libasr/codegen/asr_to_llvm.cpp", line 7698
    handle_print(x);
  File "/Users/ubaid/Desktop/OpenSource/lpython/src/libasr/codegen/asr_to_llvm.cpp", line 7993
    compute_fmt_specifier_and_arg(fmt, args, x.m_values[i], x.base.base.loc);
  File "/Users/ubaid/Desktop/OpenSource/lpython/src/libasr/codegen/asr_to_llvm.cpp", line 7812
    this->visit_expr_wrapper(v, true);
  File "/Users/ubaid/Desktop/OpenSource/lpython/src/libasr/codegen/asr_to_llvm.cpp", line 5240
    this->visit_expr(*x);
  File "../libasr/asr.h", line 5246
  File "../libasr/asr.h", line 5026
  File "/Users/ubaid/Desktop/OpenSource/lpython/src/libasr/codegen/asr_to_llvm.cpp", line 1284
    x_dict->m_key_type, x_dict->m_value_type, name2memidx);
  File "/Users/ubaid/Desktop/OpenSource/lpython/src/libasr/codegen/llvm_utils.cpp", line 3972
    rehash_all_at_once_if_needed(dict, module, key_asr_type, value_asr_type, name2memidx);
  File "/Users/ubaid/Desktop/OpenSource/lpython/src/libasr/codegen/llvm_utils.cpp", line 3934
    capacity_times_3), [&]() {
  File "../libasr/codegen/llvm_utils.h", line 349
  File "/Users/ubaid/Desktop/OpenSource/lpython/src/libasr/codegen/llvm_utils.cpp", line 3935
    rehash(dict, module, key_asr_type, value_asr_type, name2memidx);
  File "/Users/ubaid/Desktop/OpenSource/lpython/src/libasr/codegen/llvm_utils.cpp", line 3751
    llvm::Value* key_hash = get_key_hash(current_capacity, key, key_asr_type, *module);
  File "/Users/ubaid/Desktop/OpenSource/lpython/src/libasr/codegen/llvm_utils.cpp", line 3671
    throw LCompilersException("Hashing " + ASRUtils::type_to_str_python(key_asr_type) +
LCompilersException: Hashing set[i32] isn't implemented yet.

and this should print a semantic error instead of a codegen exception.

tests/errors/test_dict_key1.py Show resolved Hide resolved
@Shaikh-Ubaid Shaikh-Ubaid marked this pull request as draft April 25, 2024 19:28
@kmr-srbh
Copy link
Contributor Author

We should also check for mutable/unhashable types in the type declaration for a dict.

Let's handle this in this PR only.

@kmr-srbh kmr-srbh marked this pull request as ready for review April 26, 2024 15:06
@kmr-srbh kmr-srbh requested a review from Shaikh-Ubaid April 26, 2024 15:06
@kmr-srbh kmr-srbh force-pushed the detect-unhashable-type branch from a84486a to 53341f3 Compare April 27, 2024 05:07
Shaikh-Ubaid
Shaikh-Ubaid approved these changes Apr 27, 2024
View reviewed changes
Copy link
Collaborator

@Shaikh-Ubaid Shaikh-Ubaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shared two minor comments. Rest looks good. Thanks for this! Amazing work!

src/lpython/semantics/python_ast_to_asr.cpp Outdated Show resolved Hide resolved
src/lpython/semantics/python_ast_to_asr.cpp Outdated Show resolved Hide resolved
Shaikh-Ubaid
Shaikh-Ubaid approved these changes Apr 27, 2024
View reviewed changes
Copy link
Collaborator

@Shaikh-Ubaid Shaikh-Ubaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Thanks!

@Shaikh-Ubaid Shaikh-Ubaid enabled auto-merge (squash) April 27, 2024 08:12
@Shaikh-Ubaid Shaikh-Ubaid merged commit 921a5e3 into lcompilers:main Apr 27, 2024
14 checks passed
assem2002 pushed a commit to assem2002/lpython that referenced this pull request Apr 28, 2024
@kmr-srbh @assem2002
Detect unhashable object types at the ASR level (lcompilers#2664)
4b9ab17 
* Detect unhashable types for `dict` key

* Tests: Add error tests and update references

* Create a function to check for hashable objects

* Tests: Add error tests for `set` and update references

* Tests: Update error tests and references

* Check for unhashable types in type-annotations

* Tests: Update tests and references

* Fix indentation
assem2002 pushed a commit to assem2002/lpython that referenced this pull request Apr 28, 2024
@kmr-srbh @assem2002
Detect unhashable object types at the ASR level (lcompilers#2664)
8d0df63 
* Detect unhashable types for `dict` key

* Tests: Add error tests and update references

* Create a function to check for hashable objects

* Tests: Add error tests for `set` and update references

* Tests: Update error tests and references

* Check for unhashable types in type-annotations

* Tests: Update tests and references

* Fix indentation
@kmr-srbh kmr-srbh deleted the detect-unhashable-type branch May 2, 2024 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Shaikh-Ubaid Shaikh-Ubaid Shaikh-Ubaid approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bug: Catch unhashable objects in dict and set at the ASR level
2 participants
@kmr-srbh @Shaikh-Ubaid