modularml / mojo

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

[BUG] `def` function will output string with single quotation marks, but `fn` fucntion will not. #2207

Open jimax2000 opened 7 months ago

jimax2000 commented 7 months ago

Bug description

The same functions output different string result when using def and fn , def function will output string with single quotation marks, but fn fucntion will not.

Steps to reproduce

Here is a simple mojo file, use def and fn :

def sayHello(name):
        return "Hello, " + name + "!"

fn sayHello2(name: String) -> String:
        return "Hello, " + name + "!"

def main() :
        ret1 = sayHello("Alice")
        print("sayHello:", ret1)

        ret2 = sayHello2("Bob")
        print("sayHello2:", ret2)

        s = ret1 + ret2
        print(s)

It will output:

sayHello: 'Hello, Alice!'
sayHello2: Hello, Bob!
'Hello, Alice!'Hello, Bob!

System information

- What OS did you do install Mojo on ?
Ubuntu 22.04
- Provide version information for Mojo by pasting the output of `mojo -v`
mojo 24.2.0 (c2427bc5)
- Provide Modular CLI version by pasting the output of `modular -v`
modular 0.6.0 (04c05243)
gabrieldemarmiesse commented 6 months ago

I think that we have here a small issue in the stdlib. We use only __str__, while in python there is __str__ and __repr__. Notably it has an impact when implementing List.__str__ and Dict.__str__.

In python, we have this:

>>> print(("hello").__str__())
hello
>>> print(("hello").__repr__())
'hello'
>>> print(("hel'lo").__str__())
hel'lo
>>> print(("hel'lo").__repr__())
"hel'lo"

Notice how the quotes are different? By using only __str__ in Mojo, we can't represent well the quotes. This notably leads to those issues:

a = List[String]("hello", "world")
print(__type_of(a).__str__(a))
# [hello, world]

In python this would print ['hello', 'world']. I'll see if I can make a PR to introduce this dunder method

See the python docs which explains the requirements of both. This stackoverflow post is also very well written. I advise a read :)

In python, when using the interpreter and seeing the result of an expression, __repr__ is used, __str__ is used only when using print(). So the Mojo interpreter might want to do something similar. Same for the VScode debugger, python debuggers use __repr__().

@JoeLoser it does make sense to have the distinction no?

JoeLoser commented 6 months ago

I agree. This is "known not implemented yet" from my perspective (but no public GitHub issues exist or anything of the like unfortunately). When I originally implemented Stringable trait, str(...) and the like, this was knowingly not worried about at the time. I do think we should implement a meaningful __repr__ pretty much for all types in the standard library. At the time, it just wasn't a priority. 😄

FYI @ConnorGray in case you have thoughts here.

@walter-erquinigo @River707 for visibility with regard to what our debugger should use.

walter-erquinigo commented 6 months ago

I think that eventually we should allow repr to be used as a fallback visualizer, but not str. We'll get there eventually once we start working on expression evaluation.

ConnorGray commented 6 months ago

I don't have much to add, I agree with the argument and the context Joe added. This is something we should have, but haven't gotten to yet.

jjstankowicz commented 4 months ago

tl;rd: I think this issue could also be tangentially linked to this one [Feature Request] typeof.

Not sure this comment is super useful for development, but may be for other users who end up here. I ended up on the current issue from looking through the manual, and the basics.ipynb there shows this issue with def greet and fn greet2 methods. As someone approaching this with fresh eyes, the bigger issue for me was that I couldn't easily see the return type differences between the def greet method (where the type is completely obscured) and the fn greet2 -> String method, where the type is explicit. It was difficult to track down that def returns object types by default, but once I saw that, I was at peace (for now) with what amount to different __repr__ for the return types of the two objects. One of the attractive features of mojo is the type safety, so it was a bit jarring to run into type confusion almost instantly in the learning process.