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

Exit builtin not working #996

Closed
adamantinum opened this issue Feb 7, 2024 · 4 comments · Fixed by #998
Closed

Exit builtin not working #996

adamantinum opened this issue Feb 7, 2024 · 4 comments · Fixed by #998

Comments

@adamantinum
Copy link
Contributor

Exit builtin doesn't seem to work. The rule for Quit in mathics/builtin/evaluation.py seems correct to me, but Exit does nothing and doesn't show up among the available builtins in mathicsscript's autocompletion. Quit works just fine.
If there is anything else I could check, I would be glad to do it.

@mmatera
Copy link
Contributor

mmatera commented Feb 8, 2024

I guess that the problem is in how we load rules from builtin objects: since the pattern in the rule does not have Quit as a head, it assumes that the rule is an Upvalue, but there is no Builtin associated with the head, so at the end, the rule is not loaded.

The problem is that the loading mechanism is not showing an error when this happens.

#998 provides a patch to provide the Exit symbol, but at some point, we need to fix the Builtin.contribute mechanism.

@rocky
Copy link
Member

rocky commented Feb 8, 2024

I guess that the problem is in how we load rules from builtin objects: since the pattern in the rule does not have Quit as a head, it assumes that the rule is an Upvalue,

Are saying that this the wrong thing to do? If so, what would the right thing to do be?

I tried using a Delayed assignment and that works.

In[1]:= Exit2[n___] := Quit[n]
Out[1]= None

In[2]:= Exit2[]

Goodbye!

but there is no Builtin associated with the head, so at the end, the rule is not loaded.

The problem is that the loading mechanism is not showing an error when this happens.

I think you mean that this is another problem, that this fails silently. The first problem of course being that

    rules = {
        "Exit[n___]": "Quit[n]",
    }

doesn't work and is expected to do so.

#998 provides a patch to provide the Exit symbol, but at some point, we need to fix the Builtin.contribute mechanism.

Please add a new issue for this so it doesn't get lost. Thanks.

@mmatera
Copy link
Contributor

mmatera commented Feb 8, 2024

In my opinion, #998 would be the right way to implement Exit as a Builtin. As an alternative, we could add a WL definition in the autoload folder. This in principle is a different problem than the one I state in #1000, but #996 is the reason why I realized this is something to fix.

@rocky
Copy link
Member

rocky commented Feb 8, 2024

In my opinion, #998 would be the right way to implement Exit as a Builtin.

Ok. Is there a way to observer a difference between the two kinds of implementation in WMA? If so, which way does WMA do it?

This in principle is a different problem than the one I state in #1000, but #996 is the reason why I realized this is something to fix.

Ok.

rocky added a commit that referenced this issue Feb 10, 2024
This fixes #996. It seems that the rule for `Exit` was not loaded
together with `Quit`.

---------

Co-authored-by: R. Bernstein <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants