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] Inheritance Handling in Walker Causes Error with Non-Default Arguments #1337

Open
musab-mah-7 opened this issue Oct 3, 2024 · 1 comment
Labels
jaclang Issues related to jac programming language

Comments

@musab-mah-7
Copy link
Collaborator

Describe the bug

When using inheritance in Jac-Lang, the walker fails when non-default arguments are used after default arguments in the child walker. This is similar to the issue in Python where non-default arguments follow default arguments in data classes.

The error encountered is:

File "/home/musab/miniconda3/lib/python3.12/dataclasses.py", line 585, in _init_fn raise TypeError(f'non-default argument {f.name!r} ' TypeError: non-default argument 'new_name' follows default argument

To Reproduce

  1. Define nodes A and B with a name attribute.
  2. Create a walker visit_b_by_name that has a default argument create_b_if_not_found.
  3. Extend the walker using inheritance in get_b_by_name and update_b_by_name.
  4. Trigger the inheritance logic with a root node connected to A and B.
  5. Observe the error when trying to create the child walker with non-default arguments after default ones.

Here is the example code that causes the issue:


node A {
    has name: string;
}

node B {
    has name: string;
}

walker visit_b_by_name {
    has name: string;
    has create_b_if_not_found: bool = False;
    can visti_A with `root entry {
        visit [-->];
    }
    can visti_B with A entry {
        visit [-->(`?B)](?name in self.name) else {
            print("B not found with name: ", self.name);
        }
    }
}

walker get_b_by_name:visit_b_by_name {
    can report_B with B entry {
        print("B name: ", here.name);
        report here;
    }
}

walker update_b_by_name:visit_b_by_name {
    has new_name: string;
    can update_B with B entry {
        here.name = self.new_name;
    }
}

with entry:__main__ {
    root ++> A(name="a1") ++> B(name="b1");
    root spawn get_b_by_name(name="b1");
    root spawn update_b_by_name(name="b1", new_name="b2");
    root spawn get_b_by_name(name="b2");
}

Expected Behavior: The inheritance should allow for the proper handling of non-default arguments following default arguments, similar to how Python handles this using the kw_only=True option in data classes.

Possible Solution: In Python, we work around this by making non-default arguments keyword-only using kw_only=True in dataclass. Jac-Lang might need a similar mechanism to enforce keyword-only arguments for inherited walkers.

Workaround (if any): Currently, there doesn't appear to be a workaround within Jac-Lang itself, aside from manually reorganizing argument orders or avoiding inheritance.

Additional Context: The issue also mirrors how Python’s dataclass behaves in inheritance when default and non-default arguments are used. Here’s a similar mapping in Python:


from dataclasses import dataclass

@dataclass
class A:
    var1: int
    var2: str = "hello"

@dataclass(kw_only=True)
class B(A):
    var3: int

# Creating an instance
b_instance = B(var1=10, var3=20)

The solution in Python is to use kw_only=True in the dataclass. It would be helpful to have a similar functionality in Jac-Lang to manage inheritance behavior when mixing default and non-default arguments.

@musab-mah-7 musab-mah-7 added the jaclang Issues related to jac programming language label Oct 3, 2024
@marsninja
Copy link
Contributor

Hi @musab-mah-7, I dont think the right solution is the kw_only pattern in the lower example, upon investigation it appears a recommended approach to achieve teh desired behavior is

from dataclasses import dataclass

@dataclass
class A:
    var1: int
    var2: str = "hello"  # Optional field with default

@dataclass
class B(A):
    var3: int
    var2: str = "hello"  # Must be redefined to keep the correct order

# Creating an instance
b_instance = B(var1=10, var3=20)

print(b_instance)

Check to see if this works in Jac.

Also we can automate this semantic in our compiler as an improvement to pythons dataclasses, should be somewhat straightforward in the py-ast-gen pass using some lookups in the symbol table. @kugesan1105 what do you think? This would be a nice feature.

@marsninja marsninja removed their assignment Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jaclang Issues related to jac programming language
Projects
None yet
Development

No branches or pull requests

2 participants