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: Recursive generic definition fails to resolve #127

Closed
phorward opened this issue Dec 8, 2023 · 4 comments
Closed

bug: Recursive generic definition fails to resolve #127

phorward opened this issue Dec 8, 2023 · 4 comments
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@phorward
Copy link
Member

phorward commented Dec 8, 2023

This program

Assignment : @<Assignment, Expression, ext: void> {
    Ident _ '=' _ Expect<Assignment>  ast("assign" + ext)
    Expression
}

HoldAssignment : Assignment<HoldAssignment, Int>

HoldAssignment

generates the panic

thread 'main' panicked at src/compiler/iml/imlvalue.rs:102:27:
not yet implemented: Recursive resolve()
@phorward phorward added the bug Something isn't working label Dec 8, 2023
@phorward phorward self-assigned this Dec 9, 2023
@phorward phorward changed the title bug: Recursive resolve issue for generics bug: Recursive generic definition fails to resolve Dec 15, 2023
@phorward
Copy link
Member Author

This is a problem with the current internal ImlValue design.
fff4aa8 already marked this problems some weeks ago.
Possible solution is to remove ImlValue::Unresolved() and generally introduce a ImlRefValue, similar to ImlValue, which allows self-referencing and dynamic borrowing.

phorward added a commit that referenced this issue Dec 16, 2023
- Added tests from README, as they previously failed!
- Bug #127 in ImlValue should be fixed later, as it is a design issue
@phorward
Copy link
Member Author

a10b45a paves the way for release v0.6.5, which supports generic parselets. This issue won't be resolved for this version, and must be resolved by a future refactor or redesign. This issue is currently referenced in the panic, in case somebody gets into it.

Current resolve for the problem presented above is to define HoldAssignment as a separate parselet:

Assignment : @<Assignment, Expression, ext: void> {
    Ident _ '=' _ Expect<Assignment>  ast("assign" + ext)
    Expression
}

# workaround
HoldAssignment : @{
    Assignment<HoldAssignment, Int>
}

HoldAssignment

@phorward
Copy link
Member Author

Better example, currently tested with compiler-rework-test branch contents:

Assignment : @<Expression, Assignment: Assignment, ext: void> {
    Ident _ '=' _ Expect<Assignment>  ast("assign" + ext)
    Expression  ast("value")
}

HoldAssignment : Assignment<Int, HoldAssignment>  # endless recursion!
# HoldAssignment : Assignment<Int, HoldAssignment> Empty  # works!

# ast_print(Assignment<Int>)
ast_print(HoldAssignment)

A correct run of this program would look like this:

a=b=c=2
assign [start 1:1, end 1:8]
 assign [start 1:3, end 1:8]
  assign [start 1:5, end 1:8]
   value [start 1:7, end 1:8] => 2
a=2=3
assign [start 2:1, end 2:4]
 value [start 2:3, end 2:4] => 2
value [start 2:5, end 2:6] => 3

The problem is generally that an value_generic AST-node becomes an ImlValue::Instance, which is then directly turned into a ImlParselet. In the above example, HoldAssignment looks like a parselet, but it is just an alias for Assignment<Int, Assigment<Int, Assignment...>> with endless recursion, which cannot be resolved. Therefore, HoldAssignment must become its own parselet!

I did some tests in traverse_node_static to fix this, but it isn't satisfying enough, together with some side-problems (consumable detection, multiple error messages referring the same problem).

fn traverse_node_static(scope: &Scope, assign: Option<String>, node: &Dict) -> ImlValue {
    let emit = node["emit"].borrow();
    let emit = emit.object::<Str>().unwrap().as_str();

    if emit.starts_with("value_") && (emit != "value_generic" || assign.is_none()) {
        traverse_node_value(scope, node, assign)
    } else {
        // Handle anything else as an implicit parselet in its own scope
        let implicit_parselet = ImlParselet::new(ImlParseletInstance::new(
            None,
            None,
            traverse_node_offset(node),
            assign,
            5,
            false,
        ));

        implicit_parselet.borrow().model.borrow_mut().body = {
            match traverse_node_rvalue(
                &scope.shadow(ScopeLevel::Parselet(implicit_parselet.clone())),
                node,
                Rvalue::CallOrLoad,
            ) {
                ImlOp::Nop => return value!(void).into(),
                // Defined value call without parameters, or load becomes just the value
                ImlOp::Load { target: value, .. }
                | ImlOp::Call {
                    target: value,
                    args: None,
                    ..
                } if emit != "value_generic" => return value,

                // Any other code becomes its own parselet without any signature.
                body => body,
            }
        };

        ImlValue::from(implicit_parselet)
    }
}

@phorward phorward pinned this issue Feb 24, 2024
phorward added a commit to phorward/tokay that referenced this issue Feb 28, 2024
phorward added a commit that referenced this issue Mar 20, 2024
This isn't the correct solution, but the concept on this branch is better than before, and should be merged first.
@phorward
Copy link
Member Author

This issue is still existing and should be kept open!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant