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

Faulty Code: compiler honor failBlock #13261

Merged

Conversation

privat
Copy link
Contributor

@privat privat commented Apr 3, 2023

failBlock in OpalCompiler are used with requestor to exit the call.
Without a requestor, they were silently ignored.

The PR promote failBlock to general usage.
Having a reliable failBlock is important because it's the only way for evaluate to distinguish compile-time errors from runtime errors. Exceptions are too broad and cannot distinguish them.

Tests are also added

  • compilation with a failblock
  • evaluation with a failblock
  • code snippets that signal compilation-related exceptions (to stress the tests!)

This PR also fix a bug introduced in a previous PR with the undeclared variable GUI reparation
It required a change in the internal API parse->checkNotice

@privat privat changed the base branch from Pharo12 to Pharo11 April 3, 2023 16:43
@privat privat changed the base branch from Pharo11 to Pharo12 April 3, 2023 16:43
@privat privat force-pushed the faulty-code-compiler-failblock2 branch from e832f68 to 74e50cd Compare April 3, 2023 19:45
@MarcusDenker
Copy link
Member

Question:

Having a reliable failBlock is important because it's the only way for evaluate to distinguish compile-time errors from runtime errors. Exceptions are too broad and cannot distinguish them.

Do we have to distinguish them? I was at some point wondering if exactly the eval methods could not default to compile a faulty DoIt Method and then rely only on runtime error notification.

As eval implies that the compilation is always executed, would it really matter if the exeception is raised before or during evaluation?

@privat
Copy link
Contributor Author

privat commented Apr 4, 2023

This is a good question. I think it's important if the client (e.g. a profiler tool) wants to report back errors to the user that inputted the source.
With faulty mode and exceptions, there is no way to know if a runtime error directly come from the inputted source or from some unrelated code that is called.

@MarcusDenker MarcusDenker merged commit e67394d into pharo-project:Pharo12 Apr 4, 2023
2 of 3 checks passed
@privat
Copy link
Contributor Author

privat commented Apr 4, 2023

Error handling is always a complex matter. With this PR there is an additional solution. I'm not a fan of adding an alternative API to do somewhat the same thing. Currently, there is:

  • by default: CodeError exception at compile time, before the execution. But could be mistaken for CodeError exceptions signaled at execution-time
  • permitFaulty: compilation is always fine, and runtime errors might be signaled at execution-time. But 1. bad code that is non executed will be not considered (that might be, or might not be what the client wants) and 2. no distinction between runtime errors directly related to the bad inputted source and other possible runtime errors.
  • failBlock: compilation never signal exception but call the failBlock instead, thus error reporting is reliable. All other exceptions come from execution time (or possible internal errors of the compiler).

I think the last one covers use cases the two other cannot cover by design.
But seriously, I still don't know what is the API we should recommend for clients.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants