modularml / mojo

The Mojo Programming Language
https://docs.modular.com/mojo/manual/
Other
23.29k stars 2.59k forks source link

[BUG]: PythonObject allows conversion from arbitrary types... then crashes during elaboration #1325

Open vietanhdev opened 11 months ago

vietanhdev commented 11 months ago

Bug description

I am developing a chess engine with Mojo here: https://github.com/vietanhdev/chess.mojo/blob/96caf7bf3cebe12c626fc049cd4733d095462579/engine.mojo. My program had a problem using a Tuple as Python dictionary key.

Users/ec2-user/actions-runner/_work/modular/modular/Kernels/mojo/builtin/_startup.mojo:76:1: error: no viable expansions found
/Users/ec2-user/actions-runner/_work/modular/modular/Kernels/mojo/builtin/_startup.mojo:76:1: note:   call expansion failed - no concrete specializations
/Users/ec2-user/actions-runner/_work/modular/modular/Kernels/mojo/builtin/_startup.mojo:57:1: note:     no viable expansions found
/Users/ec2-user/actions-runner/_work/modular/modular/Kernels/mojo/builtin/_startup.mojo:71:57: note:       call expansion failed - no concrete specializations
/Users/ec2-user/actions-runner/_work/modular/modular/Kernels/mojo/builtin/_startup.mojo:32:1: note:         no viable expansions found
/Users/ec2-user/actions-runner/_work/modular/modular/Kernels/mojo/builtin/_startup.mojo:45:18: note:           call expansion failed - no concrete specializations
/Users/ec2-user/actions-runner/_work/modular/modular/Kernels/mojo/builtin/_startup.mojo:68:5: note:             no viable expansions found
/Users/ec2-user/actions-runner/_work/modular/modular/Kernels/mojo/builtin/_startup.mojo:69:22: note:               call expansion failed - no concrete specializations
/Users/vietanhdev/Works/chess.mojo/engine.mojo:537:1: note:                 no viable expansions found
def main():
^
/Users/vietanhdev/Works/chess.mojo/engine.mojo:590:102: note:                   call expansion failed - no concrete specializations
                    var moves: DynamicVector[(Int, Int, (Int, Int, StringLiteral))] = searcher.search(hist, depth)
                                                                                                     ^
/Users/vietanhdev/Works/chess.mojo/engine.mojo:482:5: note:                     no viable expansions found
    def search(inout self, history: PythonObject, depth: Int) -> DynamicVector[(Int, Int, (Int, Int, StringLiteral))]:
    ^
/Users/vietanhdev/Works/chess.mojo/engine.mojo:513:58: note:                       call expansion failed - no concrete specializations
            var move_py: PythonObject = self.tp_move.get(key)
                                                         ^
/Users/ec2-user/actions-runner/_work/modular/modular/Kernels/mojo/python/object.mojo:238:5: note:                         no viable expansions found
/Users/ec2-user/actions-runner/_work/modular/modular/Kernels/mojo/python/object.mojo:279:38: note:                           call expansion failed - no concrete specializations
/Users/ec2-user/actions-runner/_work/modular/modular/Kernels/mojo/algorithm/functional.mojo:56:1: note:                             no viable expansions found
/Users/ec2-user/actions-runner/_work/modular/modular/Kernels/mojo/algorithm/functional.mojo:67:33: note:                               call expansion failed - no concrete specializations
/Users/ec2-user/actions-runner/_work/modular/modular/Kernels/mojo/algorithm/functional.mojo:71:1: note:                                 no viable expansions found
/Users/ec2-user/actions-runner/_work/modular/modular/Kernels/mojo/algorithm/functional.mojo:78:18: note:                                   call expansion failed - no concrete specializations
/Users/ec2-user/actions-runner/_work/modular/modular/Kernels/mojo/python/object.mojo:253:9: note:                                     no viable expansions found
/Users/ec2-user/actions-runner/_work/modular/modular/Kernels/mojo/python/object.mojo:275:18: note:                                       constraint failed: cannot convert nested list element to object
mojo: error: failed to run the pass manager
```a 

### Steps to reproduce

- Download this source file and run:
https://github.com/vietanhdev/chess.mojo/blob/96caf7bf3cebe12c626fc049cd4733d095462579/engine.mojo

### System information

```shell
- What OS did you do install Mojo on ? MacOS - Macbook M1.
- Provide version information for Mojo by pasting the output of `mojo -v` mojo 0.5.0 (6e50a738)
- Provide Modular CLI version by pasting the output of `modular -v` modular 0.2.2 (95d42445)
vietanhdev commented 11 months ago

This program has the same error:

from python import Python

def main2():
    var str_a: String = "abcd"
    var x: (Int, Int, String) = (1, 2, str_a)
    var a = Python.dict()
    a[(1, 2, False)] = x
    print(a[(1, 2, False)][0].to_float64().to_int())
    a.__setitem__((1, 3, False), 12345)
    print(a.__getitem__((1, 3, False)).to_float64().to_int())
    print("hello")

fn main():
    try:
        _ = main2()
    except:
        pass

While this does not:

from python import Python

def main2():
    var str_a: StringLiteral = "abcd"
    var x: (Int, Int, StringLiteral) = (1, 2, str_a)
    var a = Python.dict()
    a[(1, 2, False)] = x
    print(a[(1, 2, False)][0].to_float64().to_int())
    a.__setitem__((1, 3, False), 12345)
    print(a.__getitem__((1, 3, False)).to_float64().to_int())
    print("hello")

fn main():
    try:
        _ = main2()
    except:
        pass

Maybe because String cannot be used as a key? Do you have any workaround?

vietanhdev commented 11 months ago

I fixed this bug by flattening all the arrays. However, I suppose that it's a bug in Mojo.

ivellapillil commented 11 months ago

Even the following fails. So does not seem to be issue of Tuple in key, but a tuple with one element being String.

from python import Python

def main():
    let s: String = "abcd"
    let x: (Int, String) = (1, s)
    let a = Python.dict()
    a["akey"] = x

The following also does not compile.

from python import Python

def main():
    let s: String = "abcd"
    let x: (String,) = (s,)
    let a = Python.dict()
    a["akey"] = x

and the following compiles:

from python import Python

def main():
    let s: String = "abcd"
    let x: (Int, Int) = (1, 2)
    let a = Python.dict()
    a["akey"] = x
ivellapillil commented 11 months ago

Seems only register_passable elements are accepted. The following fails:

from python import Python

@value
struct S: 
    var i: Int

def main():
    let s: S = 1
    let x: (S,) = (s,)
    let a = Python.dict()
    a["akey"] = x

The following passes:

from python import Python

@value
@register_passable
struct S: 
    var i: Int

def main():
    let s: S = 1
    let x: (S,) = (s,)
    let a = Python.dict()
    a["akey"] = x
jackos commented 10 months ago

This is a limitation of the builtin Tuple type as it only works with register passable types. Can potentially be resolved with traits @modularml/stdlib

JoeLoser commented 10 months ago

This is a limitation of the builtin Tuple type as it only works with register passable types. Can potentially be resolved with traits @modularml/stdlib

Yep, this is something on my radar as we look to lift the reg-passable restriction on a lot of types in the stdlib now that we have traits. Thanks for tagging me.

lattner commented 7 months ago

Tuple now works with non-register-passable values. The problem with this code:

def main():
    var s: S = 1
    var x: (S,) = (s,)
    var a = Python.dict()
    a["akey"] = x

Is that we're allowing an arbitrary conversion from any tuple of CollectionElement members to a PythonObject.

This isn't safe at all. We need to constrain this conversion to only "Pythonable" (or whatever) types that know how to convert themselves to a PythonObject.