@OddCoincidence any idea why it's failing with a segfault?

Where are you seeing a segfault?

I'm just seeing this in travis:

error[E0599]: no method named `downcast_ref` found for type `std::boxed::Box<(dyn rustpython_vm::pyobject::PyObjectPayload + 'static)>` in the current scope
   --> wasm/lib/src/browser_module.rs:174:40
    |
174 |     if let Some(promise) = obj.payload.downcast_ref::<PyPromise>() {
    |                                        ^^^^^^^^^^^^
error: aborting due to previous error

EDIT: I see it now, not sure why though...

It's in the pytest tests, command died with Signals.SIGSEGV.

Okay, so I did some debugging, here's what I found.

The new init_type_hierarchy method @adrian17 added today makes use of a FromPyObjectRef impl for PyRef<PyClass>. If that fails, it does this:

            panic!("Error getting inner type: {:?}", obj.typ)

but at that point obj.typ is uninitialized memory. So it segfaults in the debug impl.

            typ: mem::uninitialized(), // !

(So the lesson is basically that mem::uninitialized is just ridiculously hard to ever use correctly.)

So why does that impl fail? Because of this line:

        let payload: &dyn Any = &self.payload;

The concrete type for this trait object is a Box<dyn PyObjectPayload>, so when that impl tries to downcast it to a PyClass, it fails because it's actually a Box<dyn PyObjectPayload>.

Okay, I fixed it by using an as_any() method on the trait as suggested in this StackOverflow answer.

Great job figuring that out, by the way; that probably would have taken me an hour.

Oh boy, now it's a stack overflow. I didn't think this would be that hard.

Oh boy, now it's a stack overflow.

@coolreader18 I mentioned this on gitter, but in case you didn't see, I noticed that it only occurs with trace logging enabled, so that may be a good clue to help with troubleshooting this.

That's interesting, I'll look into that.

I have hit a similar issue before. It happened when trying to print Debug representation of a recursive value. This wasn't really a problem with the old PyObjectPayload as it only outputted the type of the payload rather than the value itself.

Ah, so it's probably because the typ of object_type and type_type point to each other, so it's trying to print that and going on forever. Maybe.

Yes, all the tests that are failing are ones that define some function, and it fails just after tracing with StoreName { that function }. It then tries to trace the current scope and for whatever reason a function object 's debug impl is recursive. I'll try to fix this once I get home.

Alright, found the culprit: Scope! For whatever reason trying to format the RcList in it was recursive and that caused the stack overflow. I'm not sure why this wasn't a problem before, nor am I sure how to fully fix it. For now I'm just formatting it with the string "Scope", but I think the Frame Debug impl is informative enough for locals, and that's printed after every bytecode instruction.

The remaining failures are due to recursive objects, e.g. locals() or the recursive array in test/snippets/list.py. What should we do about this? Should we use the ReprGuard for Debug formatting?

If we want a detailed Debug formatting then we kinda do need something similar to that. We probably don't want to use the same guard as it could get weird if it's printed in a repr.

I think for now it's probably fine to do like we did when we had PyObjectPayload as an enum and just have them be very basic strings, e.g "dict", "list", etc.

Rather than changing the Debug impls, maybe we can create a new method on PyValue called type_name or something and use that for the logging?

I think we do want to provide good Debug impls for tracing; just not right now, probably in a separate PR, and incrementally.