[PATCH] Improved error detection and reporting in RESTART-BIND
If you do (restart-bind ((locutus (function cdr) :report-function "23")) (error "This is an error")) you get Excessive debugger depth! Probable infinite recursion! Quitting process: #<process TOP-LEVEL>. and ECL exits. This isn't very useful. One can find various similar examples. After the patch one would get { Condition of type: SIMPLE-ERROR Macro RESTART-BIND : "23" is not a function object Available restarts: 1. (RESTART-TOPLEVEL) Go back to Top-Level REPL. Broken at SI:BYTECODES. [Evaluation of: (RESTART-BIND ((LOCUTUS #'CDR :REPORT-FUNCTION "23")) (ERROR "This is an error"))] In: #<process TOP-LEVEL>. } which is better. The new RESTART-BIND does improved error detection and reporting in other situations too. The patch follows below. You go to directory ecl-16.1.3/src/clos/ and , assuming you have saved the patch with the filename patch , you do patch -F 0 --verbose --posix conditions.lsp patch .This assumes GNU patch , otherwise skip the --verbose --posix part. I didn't make an entry in ecl-16.1.3/CHANGELOG because I'm not sure what the required format is. The file indicates near the top that it is for version 16.1.3 .So where should one mention changes which might appear in a newer version ? @===================================================================================== *** conditions.lsp Fri Jul 19 19:37:53 2019 --- conditions.lsp.new Fri Jul 19 19:39:05 2019 *************** *** 99,118 **** for n in (if (atom names) (list names) names) for f = (simple-handler-function tag i) collect (cons n f)) *handler-clusters*))) ! (defmacro restart-bind (bindings &body forms) ! `(let ((*restart-clusters* ! (cons (list ,@(mapcar #'(lambda (binding) ! `(make-restart ! :NAME ',(car binding) ! :FUNCTION ,(cadr binding) ! ,@(cddr binding))) ! bindings)) ! *restart-clusters*))) ! ,@forms)) (defun find-restart (name &optional condition) (dolist (restart (compute-restarts condition)) (when (or (eq restart name) (eq (restart-name restart) name)) (return-from find-restart restart)))) --- 99,159 ---- for n in (if (atom names) (list names) names) for f = (simple-handler-function tag i) collect (cons n f)) *handler-clusters*))) ! (defun restart-bind-check-binding ! (name foo &rest rest ! &aux int-foo rep-foo test-foo ! (err-mes (format nil "Macro ~S :" 'restart-bind))) ! (unless (symbolp name) (error "~a ~s is not a symbol" err-mes name)) ! (unless (functionp foo) (error "~a ~s is not a function object" err-mes foo)) ! (do ((br rest (cddr br))) ! ((eq br nil) (make-restart :name name :function foo ! :report-function rep-foo ! :interactive-function int-foo ! :test-function (if test-foo test-foo ! (constantly t)))) ! (let ((key (first br)) (obj (second br))) ! (unless (functionp obj) ! (error "~a ~s is not a function object" err-mes obj)) ! (cond ((eq key :interactive-function) ! (when int-foo ! (error "~a ~s given more than once" ! err-mes :interactive-function)) ! (setq int-foo obj)) ! ((eq key :report-function) ! (when rep-foo ! (error "~a ~s given more than once" ! err-mes :report-function)) ! (setq rep-foo obj)) ! ((eq key :test-function) ! (when test-foo ! (error "~a ~s given more than once" ! err-mes :test-function)) ! (setq test-foo obj)) ! (t (error "~a Inappropriate keyword ~s" err-mes key)))))) ! ! (defmacro restart-bind (bindings &body forms ! &aux (new (list 'list)) (tail new) fb eta ! (err-mes (format nil "Macro ~S :" 'restart-bind))) ! (dolist (b bindings) ! (unless (listp b) (error "~a ~s is not a list" err-mes b)) ! (setq fb (first b)) ! (unless (symbolp fb) (error "~a ~s is not a symbol" err-mes fb)) ! (unless (second b) (error "~a ~s does not have a function argument" ! err-mes b)) ! (setq eta (list 'restart-bind-check-binding `(quote ,fb) (second b))) ! (do ((l (cddr b) (cdr l)) ! (t2 (cddr eta) (cdr t2))) ! ((eql nil l)) ! (rplacd t2 (list (first l)))) ! (rplacd tail (list eta)) ! (setq tail (cdr tail))) ! `(let ((*restart-clusters* (cons ,new *restart-clusters*))) ! ,@forms)) ! (defun find-restart (name &optional condition) (dolist (restart (compute-restarts condition)) (when (or (eq restart name) (eq (restart-name restart) name)) (return-from find-restart restart))))
Hello Spiros, thank you for your contribution. We do not accept patches over mailing list. Instead we prefer merge requests on our repository. This makes possible peer review, requeesting changes and continous integration. Repository is located here: https://gitlab.com/embeddable-common-lisp/ecl Please create a git commit with detailed commit message and make a merge request there. Branch used for development is called "develop" (it will be cloned as default unless you specify it otherwise). If you are not familiar with gitlab workflow here's how it goes: - [on gitlab] create an account on the platform - [on gitlab] fork the repository into your personal userspace - [locally] clone the repository from your fork - [locally] do appriate changes on your computer and make git commit - [locally] push to your local repository - [on gitlab] go to merge requests tab and select "new merge request" While this patch is small and I could manage merging it (although it is made against ecl 16.1.3 version, so patch may fail against develop branch), I'm anxious about setting a precedence - working with patches manually is more time-consuming on my part. I hope that this explanation is sufficient to you. Once again thank you for sharing your improvements to ECL. Best regards, Daniel -- Daniel Kochmański ;; aka jackdaniel | Przemyśl, Poland TurtleWare - Daniel Kochmański | www.turtleware.eu "Be the change that you wish to see in the world." - Mahatma Gandhi
On Fri, 19 Jul 2019 22:15:42 +0200 Daniel Kochmański wrote:
thank you for your contribution. We do not accept patches over mailing list. Instead we prefer merge requests on our repository.
So If you want your code in Embeddable Common-Lisp project, please send a patch to mailing list with additional tag [PATCH] in subject. which appears in common-lisp.net/project/ecl/static/quarterly/ecl-quarterly.org is no longer valid ? I don't recall this being mentioned on the mailing list.
This makes possible peer review, requeesting changes and continous integration.
Repository is located here: https://gitlab.com/embeddable-common-lisp/ecl
Please create a git commit with detailed commit message and make a merge request there. Branch used for development is called "develop" (it will be cloned as default unless you specify it otherwise).
If you are not familiar with gitlab workflow here's how it goes: - [on gitlab] create an account on the platform - [on gitlab] fork the repository into your personal userspace - [locally] clone the repository from your fork - [locally] do appriate changes on your computer and make git commit - [locally] push to your local repository - [on gitlab] go to merge requests tab and select "new merge request"
Unfortunately gitlab.com gives at present an SSL error with all my browsers so I would have to move to a newer version of Linux before I'm able to access it and it will be a while before I get around to that. I note that I can access gitlab.common-lisp.net .I'm mentioning this because ecl-quarterly.org says After 15.3.7 release there are a few changes going on. We are now hosted at gitlab.com (however it would be nice to move to gitlab.common-lisp.net, just /not now/),
While this patch is small and I could manage merging it (although it is made against ecl 16.1.3 version, so patch may fail against develop branch), I'm anxious about setting a precedence - working with patches manually is more time-consuming on my part. I hope that this explanation is sufficient to you.
Yes , I understand. In any case it turns out that the mailing list mangled the code. At 2 places when my patch has ",@forms" , that is a comma followed by the at sign followed by "forms" , the list shows this as ", at forms" . This is probably a bug with the list software. I'm sending this message BASE64 encoded so we'll see if the comma-at sequence suffers the same transformation.
I see, in that case please make a merge request on gitlab.common-lisp.net, we'll go through peer review there and I'll merge it locally when all is fine and push to the repository we use. Here is a link to ecl repository mirror: https://gitlab.common-lisp.net/ecl/ecl
hosted at gitlab.com (however it would be nice to move to gitlab.common-lisp.net, just /not now/),
I've found writing ecl quarterly too time consuming given I work also on other projects, I wouldn't take everything I wrote then at face value. At some point I've decided that gitlab repository is good enough for time being and moving it will introduce another big confusion among contributors (and I want to avoid that unless there is a practical reason to move it again). In any case we have the mirror set up so if gitlab.com stops to be a viable solution we'll move there. Regarding formating patches (for the future), a good solution to do it (so it is applicable) is calling git format-patch after doing commits, that preserves commit messages and ensures that it is possible to apply it to the branch it was made on. Regards, Daniel -- Daniel Kochmański ;; aka jackdaniel | Przemyśl, Poland TurtleWare - Daniel Kochmański | www.turtleware.eu "Be the change that you wish to see in the world." - Mahatma Gandhi
participants (2)
-
Daniel Kochmański -
Spiros Bousbouras