From 6f095a43607506faaceedb8b22633a5770bd7f7a Mon Sep 17 00:00:00 2001 From: Christophe Rhodes Date: Tue, 13 May 2003 13:55:29 +0000 Subject: [PATCH] 0.8alpha.0.28: Fix bug 47d (DEFGENERIC must signal PROGRAM-ERROR when attempting to create a generic function with the same name as a special operator). ... sounds easy, huh? No. ... make COMPILER-ERROR not inherit from ERROR any more, so that user handlers don't (wrongly) claim to handle it; ... establish a handler for COMPILER-ERROR around the evaluator that delegates to the compiler handlers if present, but handles them itself if not... ... by signalling an error from a new internal restart, to allow user handlers for ERROR and friends a chance to run. --- BUGS | 18 -- NEWS | 3 + src/code/eval.lisp | 343 ++++++++++++++++++++------------------ src/compiler/compiler-error.lisp | 67 +++++++- src/compiler/ir1report.lisp | 2 +- tests/clos.impure.lisp | 4 +- tests/compiler.pure.lisp | 20 +-- version.lisp-expr | 2 +- 8 files changed, 261 insertions(+), 198 deletions(-) diff --git a/BUGS b/BUGS index 5a4d7c5..d9e2e60 100644 --- a/BUGS +++ b/BUGS @@ -258,24 +258,6 @@ WORKAROUND: not a binary input stream, but instead cheerfully reads from character streams, e.g. (MAKE-STRING-INPUT-STREAM "abc"). -47: - DEFCLASS bugs reported by Peter Van Eynde July 25, 2000: - d: (DEFGENERIC IF (X)) should signal a PROGRAM-ERROR, but instead - causes a COMPILER-ERROR. - -51: - miscellaneous errors reported by Peter Van Eynde July 25, 2000: - a: (PROGN - (DEFGENERIC FOO02 (X)) - (DEFMETHOD FOO02 ((X NUMBER)) T) - (LET ((M (FIND-METHOD (FUNCTION FOO02) - NIL - (LIST (FIND-CLASS (QUOTE NUMBER)))))) - (REMOVE-METHOD (FUNCTION FOO02) M) - (DEFGENERIC FOO03 (X)) - (ADD-METHOD (FUNCTION FOO03) M))) - should give an error, but SBCL allows it. - 60: The debugger LIST-LOCATIONS command doesn't work properly. diff --git a/NEWS b/NEWS index a9627a6..ee62d3f 100644 --- a/NEWS +++ b/NEWS @@ -1722,6 +1722,9 @@ changes in sbcl-0.8.0 relative to sbcl-0.8alpha.0 SB-MOP:EFFECTIVE-SLOT-DEFINITION-CLASS now have the specified-by-AMOP lambda list of (CLASS &REST INITARGS). * compiler checks for duplicated variables in macro lambda lists. + * fixed bug 47.d: (DEFGENERIC IF (X)) now signals a PROGRAM-ERROR, + not a COMPILER-ERROR (followed by some other strange error on + choosing the CONTINUE restart). * fixed some bugs revealed by Paul Dietz' test suite: ** the GENERIC-FUNCTION type is no longer disjoint from FUNCTION types. diff --git a/src/code/eval.lisp b/src/code/eval.lisp index 0ed58c2..b81bd7c 100644 --- a/src/code/eval.lisp +++ b/src/code/eval.lisp @@ -14,11 +14,17 @@ ;;; general case of EVAL (except in that it can't handle toplevel ;;; EVAL-WHEN magic properly): Delegate to #'COMPILE. (defun %eval (expr lexenv) - (funcall (sb!c:compile-in-lexenv - (gensym "EVAL-TMPFUN-") - `(lambda () - ,expr) - lexenv))) + ;; FIXME: It might be nice to quieten the toplevel by muffling + ;; warnings generated by this compilation (since we're about to + ;; execute the results irrespective of the warnings). We might want + ;; to be careful about not muffling warnings arising from inner + ;; evaluations/compilations, though [e.g. the ignored variable in + ;; (DEFUN FOO (X) 1)]. -- CSR, 2003-05-13 + (let ((fun (sb!c:compile-in-lexenv (gensym "EVAL-TMPFUN-") + `(lambda () + ,expr) + lexenv))) + (funcall fun))) ;;; Handle PROGN and implicit PROGN. (defun eval-progn-body (progn-body lexenv) @@ -52,159 +58,180 @@ (defun eval-in-lexenv (original-exp lexenv) (declare (optimize (safety 1))) ;; (aver (lexenv-simple-p lexenv)) - (let ((exp (macroexpand original-exp lexenv))) - (typecase exp - (symbol - (ecase (info :variable :kind exp) - (:constant - (values (info :variable :constant-value exp))) - ((:special :global) - (symbol-value exp)) - ;; FIXME: This special case here is a symptom of non-ANSI - ;; weirdness in SBCL's ALIEN implementation, which could - ;; cause problems for e.g. code walkers. It'd probably be - ;; good to ANSIfy it by making alien variable accessors into - ;; ordinary forms, e.g. (SB-UNIX:ENV) and (SETF SB-UNIX:ENV), - ;; instead of magical symbols, e.g. plain SB-UNIX:ENV. Then - ;; if the old magical-symbol syntax is to be retained for - ;; compatibility, it can be implemented with - ;; DEFINE-SYMBOL-MACRO, keeping the code walkers happy. - (:alien - (%eval original-exp lexenv)))) - (list - (let ((name (first exp)) - (n-args (1- (length exp)))) - (case name - ((function) - (unless (= n-args 1) - (error "wrong number of args to FUNCTION:~% ~S" exp)) - (let ((name (second exp))) - (if (and (legal-fun-name-p name) - (not (consp (let ((sb!c:*lexenv* lexenv)) - (sb!c:lexenv-find name funs))))) - (fdefinition name) - (%eval original-exp lexenv)))) - ((quote) - (unless (= n-args 1) - (error "wrong number of args to QUOTE:~% ~S" exp)) - (second exp)) - (setq - (unless (evenp n-args) - (error "odd number of args to SETQ:~% ~S" exp)) - (unless (zerop n-args) - (do ((name (cdr exp) (cddr name))) - ((null name) - (do ((args (cdr exp) (cddr args))) - ((null (cddr args)) - ;; We duplicate the call to SET so that the - ;; correct value gets returned. - (set (first args) (eval (second args)))) - (set (first args) (eval (second args))))) - (let ((symbol (first name))) - (case (info :variable :kind symbol) - ;; FIXME: I took out the *TOPLEVEL-AUTO-DECLARE* - ;; test here, and removed the *TOPLEVEL-AUTO-DECLARE* - ;; variable; the code should now act as though that - ;; variable is NIL. This should be tested.. - (:special) - (t (return (%eval original-exp lexenv)))))))) - ((progn) - (eval-progn-body (rest exp) lexenv)) - ((eval-when) - ;; FIXME: DESTRUCTURING-BIND returns ARG-COUNT-ERROR - ;; instead of PROGRAM-ERROR when there's something wrong - ;; with the syntax here (e.g. missing SITUATIONS). This - ;; could be fixed by hand-crafting clauses to catch and - ;; report each possibility, but it would probably be - ;; cleaner to write a new macro - ;; DESTRUCTURING-BIND-PROGRAM-SYNTAX which does - ;; DESTRUCTURING-BIND and promotes any mismatch to - ;; PROGRAM-ERROR, then to use it here and in (probably - ;; dozens of) other places where the same problem arises. - (destructuring-bind (eval-when situations &rest body) exp - (declare (ignore eval-when)) - (multiple-value-bind (ct lt e) - (sb!c:parse-eval-when-situations situations) - ;; CLHS 3.8 - Special Operator EVAL-WHEN: The use of - ;; the situation :EXECUTE (or EVAL) controls whether - ;; evaluation occurs for other EVAL-WHEN forms; that - ;; is, those that are not top level forms, or those in - ;; code processed by EVAL or COMPILE. If the :EXECUTE - ;; situation is specified in such a form, then the - ;; body forms are processed as an implicit PROGN; - ;; otherwise, the EVAL-WHEN form returns NIL. - (declare (ignore ct lt)) - (when e - (eval-progn-body body lexenv))))) - ((locally) - (multiple-value-bind (body decls) (parse-body (rest exp) nil) - (let ((lexenv - ;; KLUDGE: Uh, yeah. I'm not anticipating - ;; winning any prizes for this code, which was - ;; written on a "let's get it to work" basis. - ;; These seem to be the variables that need - ;; bindings for PROCESS-DECLS to work - ;; (*FREE-FUNS* and *FREE-VARS* so that - ;; references to free functions and variables in - ;; the declarations can be noted; - ;; *UNDEFINED-WARNINGS* so that warnings about - ;; undefined things can be accumulated [and then - ;; thrown away, as it happens]). -- CSR, 2002-10-24 - (let ((sb!c:*lexenv* lexenv) - (sb!c::*free-funs* (make-hash-table :test 'equal)) - (sb!c::*free-vars* (make-hash-table :test 'eq)) - (sb!c::*undefined-warnings* nil)) - (sb!c::process-decls decls - nil nil - (sb!c::make-continuation) - lexenv)))) - (eval-progn-body body lexenv)))) - ((macrolet) - (destructuring-bind (definitions &rest body) - (rest exp) - ;; FIXME: shared code with FUNCALL-IN-FOOMACROLET-LEXENV - (declare (type list definitions)) - (unless (= (length definitions) - (length (remove-duplicates definitions :key #'first))) - (style-warn "duplicate definitions in ~S" definitions)) - (let ((lexenv - (sb!c::make-lexenv - :default lexenv - :funs (mapcar - (sb!c::macrolet-definitionize-fun - :eval - ;; I'm not sure that this is the correct - ;; LEXENV to be compiling local macros - ;; in... - lexenv) - definitions)))) - (eval-in-lexenv `(locally ,@body) lexenv)))) - ((symbol-macrolet) - (destructuring-bind (definitions &rest body) - (rest exp) - ;; FIXME: shared code with FUNCALL-IN-FOOMACROLET-LEXENV - (declare (type list definitions)) - (unless (= (length definitions) - (length (remove-duplicates definitions :key #'first))) - (style-warn "duplicate definitions in ~S" definitions)) - (let ((lexenv - (sb!c::make-lexenv - :default lexenv - :vars (mapcar - (sb!c::symbol-macrolet-definitionize-fun - :eval) - definitions)))) - (eval-in-lexenv `(locally ,@body) lexenv)))) - (t - (if (and (symbolp name) - (eq (info :function :kind name) :function)) - (collect ((args)) - (dolist (arg (rest exp)) - (args (eval-in-lexenv arg lexenv))) - (apply (symbol-function name) (args))) - (%eval exp lexenv)))))) - (t - exp)))) + (handler-bind + ((sb!c:compiler-error + (lambda (c) + (if (boundp 'sb!c::*compiler-error-bailout*) + ;; if we're in the compiler, delegate either to a higher + ;; authority or, if that's us, back down to the + ;; outermost compiler handler... + (progn + (signal c) + nil) + ;; ... if we're not in the compiler, better signal a + ;; program error straight away. + (invoke-restart 'sb!c::signal-program-error))))) + (let ((exp (macroexpand original-exp lexenv))) + (typecase exp + (symbol + (ecase (info :variable :kind exp) + (:constant + (values (info :variable :constant-value exp))) + ((:special :global) + (symbol-value exp)) + ;; FIXME: This special case here is a symptom of non-ANSI + ;; weirdness in SBCL's ALIEN implementation, which could + ;; cause problems for e.g. code walkers. It'd probably be + ;; good to ANSIfy it by making alien variable accessors + ;; into ordinary forms, e.g. (SB-UNIX:ENV) and (SETF + ;; SB-UNIX:ENV), instead of magical symbols, e.g. plain + ;; SB-UNIX:ENV. Then if the old magical-symbol syntax is to + ;; be retained for compatibility, it can be implemented + ;; with DEFINE-SYMBOL-MACRO, keeping the code walkers + ;; happy. + (:alien + (%eval original-exp lexenv)))) + (list + (let ((name (first exp)) + (n-args (1- (length exp)))) + (case name + ((function) + (unless (= n-args 1) + (error "wrong number of args to FUNCTION:~% ~S" exp)) + (let ((name (second exp))) + (if (and (legal-fun-name-p name) + (not (consp (let ((sb!c:*lexenv* lexenv)) + (sb!c:lexenv-find name funs))))) + (fdefinition name) + (%eval original-exp lexenv)))) + ((quote) + (unless (= n-args 1) + (error "wrong number of args to QUOTE:~% ~S" exp)) + (second exp)) + (setq + (unless (evenp n-args) + (error "odd number of args to SETQ:~% ~S" exp)) + (unless (zerop n-args) + (do ((name (cdr exp) (cddr name))) + ((null name) + (do ((args (cdr exp) (cddr args))) + ((null (cddr args)) + ;; We duplicate the call to SET so that the + ;; correct value gets returned. + (set (first args) (eval (second args)))) + (set (first args) (eval (second args))))) + (let ((symbol (first name))) + (case (info :variable :kind symbol) + ;; FIXME: I took out the *TOPLEVEL-AUTO-DECLARE* + ;; test here, and removed the + ;; *TOPLEVEL-AUTO-DECLARE* variable; the code + ;; should now act as though that variable is + ;; NIL. This should be tested.. + (:special) + (t (return (%eval original-exp lexenv)))))))) + ((progn) + (eval-progn-body (rest exp) lexenv)) + ((eval-when) + ;; FIXME: DESTRUCTURING-BIND returns ARG-COUNT-ERROR + ;; instead of PROGRAM-ERROR when there's something wrong + ;; with the syntax here (e.g. missing SITUATIONS). This + ;; could be fixed by hand-crafting clauses to catch and + ;; report each possibility, but it would probably be + ;; cleaner to write a new macro + ;; DESTRUCTURING-BIND-PROGRAM-SYNTAX which does + ;; DESTRUCTURING-BIND and promotes any mismatch to + ;; PROGRAM-ERROR, then to use it here and in (probably + ;; dozens of) other places where the same problem + ;; arises. + (destructuring-bind (eval-when situations &rest body) exp + (declare (ignore eval-when)) + (multiple-value-bind (ct lt e) + (sb!c:parse-eval-when-situations situations) + ;; CLHS 3.8 - Special Operator EVAL-WHEN: The use of + ;; the situation :EXECUTE (or EVAL) controls whether + ;; evaluation occurs for other EVAL-WHEN forms; that + ;; is, those that are not top level forms, or those + ;; in code processed by EVAL or COMPILE. If the + ;; :EXECUTE situation is specified in such a form, + ;; then the body forms are processed as an implicit + ;; PROGN; otherwise, the EVAL-WHEN form returns NIL. + (declare (ignore ct lt)) + (when e + (eval-progn-body body lexenv))))) + ((locally) + (multiple-value-bind (body decls) (parse-body (rest exp) nil) + (let ((lexenv + ;; KLUDGE: Uh, yeah. I'm not anticipating + ;; winning any prizes for this code, which was + ;; written on a "let's get it to work" basis. + ;; These seem to be the variables that need + ;; bindings for PROCESS-DECLS to work + ;; (*FREE-FUNS* and *FREE-VARS* so that + ;; references to free functions and variables + ;; in the declarations can be noted; + ;; *UNDEFINED-WARNINGS* so that warnings about + ;; undefined things can be accumulated [and + ;; then thrown away, as it happens]). -- CSR, + ;; 2002-10-24 + (let ((sb!c:*lexenv* lexenv) + (sb!c::*free-funs* (make-hash-table :test 'equal)) + (sb!c::*free-vars* (make-hash-table :test 'eq)) + (sb!c::*undefined-warnings* nil)) + (sb!c::process-decls decls + nil nil + (sb!c::make-continuation) + lexenv)))) + (eval-progn-body body lexenv)))) + ((macrolet) + (destructuring-bind (definitions &rest body) + (rest exp) + ;; FIXME: shared code with + ;; FUNCALL-IN-FOOMACROLET-LEXENV + (declare (type list definitions)) + (unless (= (length definitions) + (length (remove-duplicates definitions + :key #'first))) + (style-warn "duplicate definitions in ~S" definitions)) + (let ((lexenv + (sb!c::make-lexenv + :default lexenv + :funs (mapcar + (sb!c::macrolet-definitionize-fun + :eval + ;; I'm not sure that this is the + ;; correct LEXENV to be compiling + ;; local macros in... + lexenv) + definitions)))) + (eval-in-lexenv `(locally ,@body) lexenv)))) + ((symbol-macrolet) + (destructuring-bind (definitions &rest body) + (rest exp) + ;; FIXME: shared code with + ;; FUNCALL-IN-FOOMACROLET-LEXENV + (declare (type list definitions)) + (unless (= (length definitions) + (length (remove-duplicates definitions + :key #'first))) + (style-warn "duplicate definitions in ~S" definitions)) + (let ((lexenv + (sb!c::make-lexenv + :default lexenv + :vars (mapcar + (sb!c::symbol-macrolet-definitionize-fun + :eval) + definitions)))) + (eval-in-lexenv `(locally ,@body) lexenv)))) + (t + (if (and (symbolp name) + (eq (info :function :kind name) :function)) + (collect ((args)) + (dolist (arg (rest exp)) + (args (eval-in-lexenv arg lexenv))) + (apply (symbol-function name) (args))) + (%eval exp lexenv)))))) + (t + exp))))) ;;; miscellaneous full function definitions of things which are ;;; ordinarily handled magically by the compiler diff --git a/src/compiler/compiler-error.lisp b/src/compiler/compiler-error.lisp index 0c5e751..e0d3def 100644 --- a/src/compiler/compiler-error.lisp +++ b/src/compiler/compiler-error.lisp @@ -22,8 +22,7 @@ ;;; a function that is called to unwind out of COMPILER-ERROR (declaim (type (function () nil) *compiler-error-bailout*)) -(defvar *compiler-error-bailout* - (lambda () (error "COMPILER-ERROR with no bailout"))) +(defvar *compiler-error-bailout*) ;;; an application programmer's error caught by the compiler ;;; @@ -33,7 +32,51 @@ ;;; and turned into diagnostic output and a FAILURE-P return value ;;; from COMPILE or COMPILE-FILE. Bugs in SBCL itself throw us into ;;; the debugger. -(define-condition compiler-error (simple-error) ()) +;;; +;;; A further word or two of explanation might be warranted here, +;;; since I (CSR) have spent the last day or so wandering in a +;;; confused daze trying to get this to behave nicely before finally +;;; hitting on the right solution. +;;; +;;; These objects obey a slightly involved protocol in order to +;;; achieve the right dynamic behaviour. If we signal a +;;; COMPILER-ERROR from within the compiler, we want that the +;;; outermost call to COMPILE/COMPILE-FILE cease attempting to compile +;;; the code in question and instead compile a call to signal a +;;; PROGRAM-ERROR. This is achieved by resignalling the condition +;;; from within the handler, so that the condition travels up the +;;; handler stack until it finds the outermost handler. Why the +;;; outermost? Well, COMPILE-FILE could call EVAL from an EVAL-WHEN, +;;; which could recursively call COMPILE, which could then signal an +;;; error; we want the inner EVAL not to fail so that we can go on +;;; compiling, so it's the outer COMPILE-FILE that needs to replace +;;; the erroneous call with a call to ERROR. +;;; +;;; This resignalling up the stack means that COMPILER-ERROR should +;;; not be a generalized instance of ERROR, as otherwise code such as +;;; (IGNORE-ERRORS (DEFGENERIC IF (X))) will catch and claim to handle +;;; the COMPILER-ERROR. So we make COMPILER-ERROR inherit from +;;; SIMPLE-CONDITION and SERIOUS-CONDITION instead, as of +;;; sbcl-0.8alpha.0.2x, so that unless the user claims to be able to +;;; handle SERIOUS-CONDITION (and if he does, he deserves what's going +;;; to happen :-) +;;; +;;; So, what if we're not inside the compiler, then? Well, in that +;;; case we're in the evaluator, so we want to convert the +;;; COMPILER-ERROR into a PROGRAM-ERROR and signal it immediately. We +;;; have to signal the PROGRAM-ERROR from the dynamic environment of +;;; attempting to evaluate the erroneous code, and not from any +;;; exterior handler, so that user handlers for PROGRAM-ERROR and +;;; ERROR stand a chance of running, in e.g. (IGNORE-ERRORS +;;; (DEFGENERIC IF (X))). So this is where the SIGNAL-PROGRAM-ERROR +;;; restart comes in; the handler in EVAL-IN-LEXENV chooses this +;;; restart if it believes that the compiler is not present (which it +;;; tests using the BOUNDPness of *COMPILER-ERROR-BAILOUT*). The +;;; restart executes in the dynamic environment of the original +;;; COMPILER-ERROR call, and all is well. +;;; +;;; CSR, 2003-05-13 +(define-condition compiler-error (simple-condition serious-condition) ()) ;;; Signal the appropriate condition. COMPILER-ERROR calls the bailout ;;; function so that it never returns (but compilation continues). @@ -47,12 +90,18 @@ :format-control format-string :format-arguments format-args)) (defun compiler-error (format-string &rest format-args) - (cerror "Replace form with call to ERROR." - 'compiler-error - :format-control format-string - :format-arguments format-args) - (funcall *compiler-error-bailout*) - (bug "Control returned from *COMPILER-ERROR-BAILOUT*.")) + (restart-case + (progn + (cerror "Replace form with call to ERROR." + 'compiler-error + :format-control format-string + :format-arguments format-args) + (funcall *compiler-error-bailout*) + (bug "Control returned from *COMPILER-ERROR-BAILOUT*.")) + (signal-program-error () + (error 'simple-program-error + :format-control format-string + :format-arguments format-args)))) (defun compiler-warn (format-string &rest format-args) (apply #'warn format-string format-args) (values)) diff --git a/src/compiler/ir1report.lisp b/src/compiler/ir1report.lisp index c8dced2..4ef2d65 100644 --- a/src/compiler/ir1report.lisp +++ b/src/compiler/ir1report.lisp @@ -400,7 +400,7 @@ (what (etypecase condition (style-warning 'style-warning) (warning 'warning) - (error 'error)))) + ((or error compiler-error) 'error)))) (multiple-value-bind (format-string format-args) (if (typep condition 'simple-condition) (values (simple-condition-format-control condition) diff --git a/tests/clos.impure.lisp b/tests/clos.impure.lisp index 14787f5..2f642b5 100644 --- a/tests/clos.impure.lisp +++ b/tests/clos.impure.lisp @@ -320,7 +320,9 @@ (assert-program-error (defclass foo008 () (a :initarg :a) (:default-initargs :a 1) - (:default-initargs :a 2)))) + (:default-initargs :a 2))) + ;; and also BUG 47d, fixed in sbcl-0.8alpha.0.26 + (assert-program-error (defgeneric if (x)))) ;;; DOCUMENTATION's argument-precedence-order wasn't being faithfully ;;; preserved through the bootstrap process until sbcl-0.7.8.39. diff --git a/tests/compiler.pure.lisp b/tests/compiler.pure.lisp index c8f2de6..74c91db 100644 --- a/tests/compiler.pure.lisp +++ b/tests/compiler.pure.lisp @@ -75,16 +75,16 @@ ;;; another LET-related bug fixed by Alexey Dejneka at the same ;;; time as bug 112 -(multiple-value-bind (value error) - (ignore-errors - ;; should complain about duplicate variable names in LET binding - (compile nil - '(lambda () - (let (x - (x 1)) - (list x))))) - (assert (null value)) - (assert (typep error 'error))) +(multiple-value-bind (fun warnings-p failure-p) + ;; should complain about duplicate variable names in LET binding + (compile nil + '(lambda () + (let (x + (x 1)) + (list x)))) + (declare (ignore warnings-p)) + (assert (functionp fun)) + (assert failure-p)) ;;; bug 169 (reported by Alexey Dejneka 2002-05-12, fixed by David ;;; Lichteblau 2002-05-21) diff --git a/version.lisp-expr b/version.lisp-expr index f9bbf13..43ed933 100644 --- a/version.lisp-expr +++ b/version.lisp-expr @@ -17,4 +17,4 @@ ;;; checkins which aren't released. (And occasionally for internal ;;; versions, especially for internal versions off the main CVS ;;; branch, it gets hairier, e.g. "0.pre7.14.flaky4.13".) -"0.8alpha.0.27" +"0.8alpha.0.28" -- 1.7.10.4