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.

Sign up for GitHub

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

Jump to bottom

[Meta-issue] improve faulty compiling #12883

Open
51 of 78 tasks
privat opened this issue Feb 28, 2023 · 1 comment
Open
51 of 78 tasks

[Meta-issue] improve faulty compiling #12883

privat opened this issue Feb 28, 2023 · 1 comment
Labels
Project: Compiler

Comments

@privat
Copy link
Contributor

privat commented Feb 28, 2023 •
edited
Loading

Parsing and compiling is an everyday job and is performed when you install a package, switch branch, compile a method or add a character in a method editor (because of highlighting).

AST-Code (and its client) and OpalCompiler have/had a limited approach when dealing with faulty code.

The historical approach was basically to fire the UI in case of code error, so the human can fix it (in a not-so interactive way).
This approach is somewhat limited if tools have to work during code edition (or on a random piece of code) to provide better user feedback.

Pharo included basic opt-in faulty parsing (and compiler related options) so AST clients can try to deal with faulty code (highlighting for instance). This Issue track improvement and discussions.

Bad API

The main issue of the current API are:

  • a lack of isolation with UI: magic requestor instance variable, compiler drives the UI presentation on syntax error or semantic error (with interactive menu)
    → mostly eliminated. Some requestor thing remains, but I wait for open season in spec/newtools repositories
  • a bloated code recovery: notification ping-pong (SyntaxErrorNotification, OCUndeclaredVariableWarning, ReparseAfterSourceEditing).
    → Now fixed
  • complex effects on the configuration: requestor and its interactiveness, optionParseErrors, optionParseErrorsNoInteractive. optionSkipSemanticWarnings
    → Now fixed
  • lack of robustness: fragile errorblock and failBlock, unclear exception signaler, unclear notification signaled and possibly resumed
    → Now fixed

Current debugger GUI on faulty code

Some notes on error recovery. i.e. what happens when a faulty node is executed.

Evaluate expression (DoIt method)

Smalltalk compiler permitFaulty: true; evaluate: '1+¿'.
  • launch a debugger with RuntimeSyntaxError: Unknown character
  • sane stack trace (nil>>DoIt, OC>>evaluateDoIt, OC>>evaluate, OC>>evaluate:)
  • present original code
  • visible cursor and error located in the code
  • recovery. no way to recover from here, cannot modify a doit method, but it is a limitation of the debugger, not ours, so I add a checkmark

Execute non installed method

nil executeMethod: (Smalltalk compiler permitFaulty: true; compile: 'foo ^ 1+¿')
  • launch a debugger with RuntimeSyntaxError: Unknown character
  • sane stack trace (nil>>foo, nil>>executeMethod:)
  • present original code.
  • visible cursor and error located in the code
  • recovery. no way to recover from here. can modify method but key #foo not found in MethodDictionary Debugger should prevent reinstallation on non-installed method #12931. So a bug of the debugger, not ours, so I add a checkmark

Execute installed method

nil class compiler permitFaulty: true; install: 'foo ^ 1+¿'. nil foo.
  • launch a debugger with RuntimeSyntaxError: Unknown character
  • sane stack trace (nil>>foo)
  • present original code.
  • visible cursor and error located in the code
  • can recover by removing the ¿ replacing it by a valid expression, saving, and proceed

Installing and committing faulty code

How far can faulty code goes?
Related issues and experiments: #11944 #13424

  • installing a faulty method. OK.
    Note this require direct call to OpalCompiler or other MOP facility.
    Editors won't let you install any faulty methods.
  • Creating a commit.
    Iceberg has no issue to commit the method.
    However, there is a #error critique ReMethodHasSyntaxErrorRule, so you cannot push faulty code unwillingly :)
    • Can faulty code mess with the serialization format (tonel or whatever) and corrupt the file?
      Note that file corruption should already be handled, as bad command-line file edition can happen.
      But I don't know is the serialization format is fragile if the method contains garbage.
      → I checked, It seems that yes, the format is very fragile. It uses an ad hoc scanner and expect the content to be well-formed. This is bad because 1. it is uselessly complex code 2. it forces it to be synchronized with the language features (e.g. new literals or something) 3. it's limiting.
      → Update: PR proposed: Failsafe tonel pharo-vcs/tonel#108
  • Removing the faulty method (switch to a sane branch). Unloading the code was ok.
  • Checkout back the branch with the faulty method
    • The diff is shown in the "preview", with the syntax error visible in the diff, but nothing is done to alert you.
      However, up to my knowledge, Iceberg never tries to warn you on the diff anyway. So full check mark here.
    • Proceeding "checkout" works until it shows a CodeError debugger.
      Inspection of the syntax error is "manual" but easy (just inspect self). While it could be improved, it is much better than the awful syntax error debugger I removed at the beginning of Pharo12 (Kill syntax error debugger #12910). So full check mark here.
    • Closing the debugger, cancel the checkout.
      Since there is nothing else in the commit, I don't know if the system is left in the middle or if there is some kind of transaction/rollback. However, this is a "classic" iceberg issue of fault handling and not specifically related to faulty code with syntax errors, so full check mark here
    • Proceeding "checkout" again, bring the same debugger, Now I "proceed ▶️" (the button is green because CodeError are resumable) and the faulty method is installed. The idea is that you can fix it at your own pace.
  • Code change. Go to Epicea and try to "Revert..." the faulty method.
    Epicea show you the diff and ask you to confirm. All fine and it revert the bad method
  • Return to the code change. Try to "Apply..." the faulty method.
    Epicea show you the diff and ask you to confirm. But a growl (popup infobox in the lower left corner of the screen) show you the description of the CodeError, instead of launching a debugger. Why? Why you failed me Epicea?
    fixed by EpiceaBrowsers: stop catching all errors #13425

I'm quite happy about the current status.

List of things
@privat privat mentioned this issue Feb 28, 2023
Open
@jecisc
Copy link
Member

jecisc commented Mar 1, 2023

New one :)

#12886

@privat privat mentioned this issue Mar 5, 2023
Merged
@privat privat mentioned this issue Mar 14, 2023
Closed
6 tasks
This was referenced Mar 28, 2023
Merged
Closed
@privat privat changed the title Meta-issue: improve faulty compiling [Meta-issue] improve faulty compiling Mar 31, 2023
@MarcusDenker MarcusDenker moved this to In Progress in Pharo12 Apr 10, 2023
@privat privat mentioned this issue Apr 13, 2023
Merged
@MarcusDenker MarcusDenker moved this to 🆕 New in Compiler and AST May 11, 2023
@MarcusDenker MarcusDenker moved this from 🆕 New to 🏗 In progress in Compiler and AST May 11, 2023
@MarcusDenker MarcusDenker moved this from In Progress to Todo in Pharo12 Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Project: Compiler
Projects
Compiler and AST
Status: 🏗 In progress
Pharo12
Status: Todo
Milestone
No milestone
Development

No branches or pull requests
2 participants
@privat @jecisc