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.

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

[BUG]: Modifying nested element of owned DynamicVector crashes during parsing #1546

Closed
isuckatcs opened this issue Dec 22, 2023 · 3 comments
Closed
Labels
bug Something isn't working crash mojo Issues that are related to mojo mojo-lang Tag for all issues related to language.

Comments

@isuckatcs
Copy link
Contributor

Bug description

The following snippet crashes during parsing/sema.

from collections.vector import DynamicVector


@value
@register_passable
struct Nested:
    var x: Int


@value
@register_passable
struct Wrapper(CollectionElement):
    var n: Nested


fn foo(owned v: DynamicVector[Wrapper]):
    v[0].n.x = 0
mojo: /__w/modular/modular/KGEN/lib/MojoParser/ExprEmitter.cpp:769: M::KGEN::LIT::SRValue M::KGEN::LIT::ExprEmitter::emitSRValue(ASTExprAnd<M::KGEN::LIT::AnyValue>, M::KGEN::LIT::ExprContext, M::KGEN::LIT::ASTType): Assertion `pValue && "must be PValue if register-passable and not SRValue"' failed.
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 repro.mojo
1.      Crash resolving decl body at loc(".../repro.mojo":16:1)
    >> fn foo(owned v: DynamicVector[Wrapper]):
       ^.......................................
    >>     v[0].n.x = 0
       ................<
2.      Crash parsing statement at loc(".../repro.mojo":17:5)
    >>     v[0].n.x = 0
           ^...........<

Steps to reproduce

  • run the snippet in the description with mojo

System information

- Ubuntu 22.04.3 LTS (WSL 2)
- mojo 0.6.1 (876ded2e)
- modular 0.3.1 (589ce200)
@isuckatcs isuckatcs added bug Something isn't working mojo Issues that are related to mojo labels Dec 22, 2023
@duckki
Copy link

duckki commented Dec 30, 2023

I think @value and @register_passable are not compatible each other, since @register_passable requires different style of constructors than what @value would added.

A workaround might be using @register_passable("trivial").

(Indeed, the crash persists even with @register_passable("trivial"). Please disregard my suggestion.)

@isuckatcs
Copy link
Contributor Author

So, while playing around with @register_passable, I discovered #1579.

I modified the snippet such that the required constructors are specified manually, the way @register_passable requires them to be (based on the compiler error messages), and v[0].n.x = 0 still causes a crash.

@register_passable
struct Nested:
    var x: Int

    fn __init__() -> Nested:
        return Self {x: 0}

    fn __copyinit__(other: Self) -> Nested:
        return Self {x: other.x}


@register_passable
struct Wrapper(CollectionElement):
    var n: Nested

    fn __init__() -> Wrapper:
        return Self {n: Nested()}

    fn __copyinit__(other: Self) -> Wrapper:
        return Self {n: other.n}


fn foo(owned v: DynamicVector[Wrapper]):
    v[0].n.x = 0
mojo: /__w/modular/modular/KGEN/lib/MojoParser/ExprEmitter.cpp:769: M::KGEN::LIT::SRValue M::KGEN::LIT::ExprEmitter::emitSRValue(ASTExprAnd<M::KGEN::LIT::AnyValue>, M::KGEN::LIT::ExprContext, M::KGEN::LIT::ASTType): Assertion `pValue && "must be PValue if register-passable and not SRValue"' failed.

Adding @register_passable("trivial") still results in the same crash regardless of whether @value is present or not.

@register_passable("trivial")
struct Nested:
    var x: Int


@register_passable("trivial")
struct Wrapper(CollectionElement):
    var n: Nested


fn foo(owned v: DynamicVector[Wrapper]):
    v[0].n.x = 0
mojo: /__w/modular/modular/KGEN/lib/MojoParser/ExprEmitter.cpp:769: M::KGEN::LIT::SRValue M::KGEN::LIT::ExprEmitter::emitSRValue(ASTExprAnd<M::KGEN::LIT::AnyValue>, M::KGEN::LIT::ExprContext, M::KGEN::LIT::ASTType): Assertion `pValue && "must be PValue if register-passable and not SRValue"' failed.

@ematejska ematejska added the mojo-lang Tag for all issues related to language. label Jan 3, 2024
@Mogball
Copy link
Collaborator

Mogball commented Feb 22, 2024

This has been fixed at top-of-tree.

@Mogball Mogball closed this as completed Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working crash mojo Issues that are related to mojo mojo-lang Tag for all issues related to language.
Projects
None yet
Development

No branches or pull requests

4 participants