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

Add lastError function #3654

Draft
wants to merge 8 commits into
base: development
Choose a base branch
from

Conversation

d-torrance
Copy link
Member

This returns a sequence containing the location and message string for the last error (at least most of the time, e.g., not for syntax errors).

The eventual goal is to use this in the MPI package (see #2129) for handling errors gracefully, e.g., doing something like try x else lastError().

d-torrance and others added 8 commits February 9, 2025 15:17
Sets lastError, so use it for constructing all errors except control
transferring errors like return, continue, etc.
Also set position for suppressed errors
In particular, we previously constructed the error message when
returning with it (when stopIfError/returnIfError true).  Now we
always construct it (so it gets saved to lastError), but only return
with it in those cases.
Also remove a bunch of cruft in the file error-messages.m2
e(x,p1,h1)
f(x,p1,h1)
g(x,p1,h1)
assert((last try 1/0 else lastError()) == "division by zero")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you planning on replacing the manual tests above with automatic versions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea!

@pzinn
Copy link
Contributor

pzinn commented Feb 13, 2025

I did something similar on my dev branch. I'll compare the code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not directly related, but do you know what these two lines are for:

M2/M2/Macaulay2/d/expr.d

Lines 307 to 308 in 3fc8759

export errorMessageClass := newtypeof(basicListClass);
export missingMethodClass := newtypeof(errorMessageClass);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea! They don't appeared to be used anywhere.

@pzinn
Copy link
Contributor

pzinn commented Feb 26, 2025

here are AFAICT the main differences I find between your implementation and mine:

  • I update the last error message when it's printed, not when it's built.
  • I define a new global variable lastError (well, I didn't call it that...) rather than a function lastError(). On that last point, can you comment on why this is better?

@d-torrance
Copy link
Member Author

  • I update the last error message when it's printed, not when it's built.

One of the big goals was to find out if there was an error without actually raising one, e.g., try foo() else lastError() would tell us what happened if foo() raised and error without actually raising an error.

That being said, @MichaelABurr has some code updating it during printErrorMessage. See d-torrance#13. Is that what you were doing, too?

  • I define a new global variable lastError (well, I didn't call it that...) rather than a function lastError(). On that last point, can you comment on why this is better?

That was our original idea, but we didn't realize it was possible. Most of the global variables like this (e.g., printWidth) seem to be set at top-level and accessed in the interpreter. Whereas this would be set in the interpreter and accessed at top-level. We couldn't figure out an obvious way to do that, hence the function.

Is your code live on Macaulay2Web? Or just in a branch somewhere?

@pzinn
Copy link
Contributor

pzinn commented Feb 26, 2025

good point. maybe I'm wrong about updating it when it's printed, as you said, the main goal is to test the error message with try. Yes it is live on M2Web, but it's not working any longer, I made this at the same time as my (failed) error upgrade exactly a year ago, and since then changes to the official development branch broke it; try viewHelp "try", the example there doesn't work any more.

@pzinn
Copy link
Contributor

pzinn commented Feb 26, 2025

the relevant commit seems to be 74e022a
yeah it's clumsy, it updates the error message when it's printed or when try is called (which I'm assuming is the part that got overwritten by a subsequent PR).

@pzinn
Copy link
Contributor

pzinn commented Feb 26, 2025

I still think having a global variable is marginally simpler than a function.
Ah, another thing is I also defined the location of the last error. Not sure if it belongs in your PR.
The goal for me was to transfer all the error printing to the m2 level.

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 this pull request may close these issues.

4 participants