Re: [Emacs-diffs] emacs-25 db436e9: Don't call debug on failed cl-assert

Re: [Emacs-diffs] emacs-25 db436e9: Don't call debug on failed cl-assert

[Stefan Monnier]

>     Don't call debug on failed cl-assert

That's really too bad: I introduced it very explicitly because I found
it extremely convenient.

> Doing this causes problems when running ert tests

Can it be fixed somewhere else?


        Stefan

Re: [Emacs-diffs] emacs-25 db436e9: Don't call debug on failed cl-assert

[Noam Postavsky]

On Sat, Nov 5, 2016 at 1:22 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
> >     Don't call debug on failed cl-assert
>
> That's really too bad: I introduced it very explicitly because I found
> it extremely convenient.

Oh, I see; it would be helpful if you had included the rational in the
commit message where you introduced it.

>
> > Doing this causes problems when running ert tests
>
> Can it be fixed somewhere else?

I don't quite understand what's the benefit of calling debug when
debug-on-error is non-nil: signalling the error is going to call it
anyway.

But it looks like funcalling `debugger' instead of calling `debug'
seems to work:

(defun cl--assertion-failed (form &optional string sargs args)
  (if debug-on-error
      (funcall debugger `(cl-assertion-failed ,form ,string ,@sargs))
    (if string
        (apply #'error string (append sargs args))
      (signal 'cl-assertion-failed `(,form ,@sargs)))))

Re: [Emacs-diffs] emacs-25 db436e9: Don't call debug on failed cl-assert

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 468 bytes --]

Stefan Monnier <monnier@iro.umontreal.ca> schrieb am So., 6. Nov. 2016 um
18:39 Uhr:

> > Doing this causes problems when running ert tests
>
> Can it be fixed somewhere else?
>
>
ERT should not have to install a debugger to do its job. Rather,
condition-case should be extended to be able to catch all signals (
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=24618) and should get direct
access to the backtrace (https://debbugs.gnu.org/cgi/bugreport.cgi?bug=24617
).

[-- Attachment #2: Type: text/html, Size: 965 bytes --]

Re: [Emacs-diffs] emacs-25 db436e9: Don't call debug on failed cl-assert

[Stefan Monnier]

> Oh, I see; it would be helpful if you had included the rational in the
> commit message where you introduced it.

Indeed.

>> > Doing this causes problems when running ert tests
>> Can it be fixed somewhere else?
> I don't quite understand what's the benefit of calling debug when
> debug-on-error is non-nil: signalling the error is going to call it
> anyway.

Not if the code is run within an `ignore-errors` clause or some other
condition-case catching `error`.

> But it looks like funcalling `debugger' instead of calling `debug'
> seems to work:
> (defun cl--assertion-failed (form &optional string sargs args)
>   (if debug-on-error
>       (funcall debugger `(cl-assertion-failed ,form ,string ,@sargs))

Works for me.


        Stefan

Re: [Emacs-diffs] emacs-25 db436e9: Don't call debug on failed cl-assert

[Noam Postavsky]

On Sun, Nov 6, 2016 at 5:47 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
>> I don't quite understand what's the benefit of calling debug when
>> debug-on-error is non-nil: signalling the error is going to call it
>> anyway.
>
> Not if the code is run within an `ignore-errors` clause or some other
> condition-case catching `error`.

Hmm, this seems like kind of a kludgy way to circumvent ignore-errors.
Wouldn't it better to use
(setq debug-on-error '(cl-assertion-failed) debug-on-signal t) along with:

(defun cl--assertion-failed (form &optional string sargs args)
  (signal 'cl-assertion-failed
          (if string (apply #'format-message string (append sargs args))
            `(,form ,@sargs))))

Re: [Emacs-diffs] emacs-25 db436e9: Don't call debug on failed cl-assert

[Stefan Monnier]

>> Not if the code is run within an `ignore-errors` clause or some other
>> condition-case catching `error`.
> Hmm, this seems like kind of a kludgy way to circumvent ignore-errors.

It's not perfect, indeed.

> Wouldn't it better to use
> (setq debug-on-error '(cl-assertion-failed) debug-on-signal t) along with:

But I want debug-on-error to be t, so setting debug-on-signal non-nil is
not an option because it's too invasive.

Another option would be to declare that the `cl-assertion-failed` signal
does not have `error` as its parent, so ignore-errors wouldn't catch it.
But really, this should only be the case when debug-on-error is non-nil,
which is rather problematic.


        Stefan

Re: [Emacs-diffs] emacs-25 db436e9: Don't call debug on failed cl-assert

[Stefan Monnier]

>> Wouldn't it better to use
>> (setq debug-on-error '(cl-assertion-failed) debug-on-signal t) along with:
> But I want debug-on-error to be t, so setting debug-on-signal non-nil is
> not an option because it's too invasive.

Hmm... maybe what I'd really want is

   (setq debug-on-signal '(cl-assertion-failed))


-- Stefan

Re: [Emacs-diffs] emacs-25 db436e9: Don't call debug on failed cl-assert

[Noam Postavsky]

On Tue, Nov 8, 2016 at 9:32 AM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>>> Wouldn't it better to use
>>> (setq debug-on-error '(cl-assertion-failed) debug-on-signal t) along with:
>> But I want debug-on-error to be t, so setting debug-on-signal non-nil is
>> not an option because it's too invasive.
>
> Hmm... maybe what I'd really want is
>
>    (setq debug-on-signal '(cl-assertion-failed))

That makes sense. I think we should do that for 26.1. Meanwhile, I've
pushed the (funcall debugger...) solution to emacs-25 since that is
less of a change vs 25.1 than what's there currently.

Re: [Emacs-diffs] emacs-25 db436e9: Don't call debug on failed cl-assert

[Clément Pit-Claudel]

On 2016-11-06 14:03, Philipp Stephani wrote:
> ERT should not have to install a debugger to do its job. Rather, 
> condition-case[...] should get direct access to the backtrace 
> (https://debbugs.gnu.org/cgi/bugreport.cgi?bug=24617).

That would be wonderful.

Re: [Emacs-diffs] emacs-25 db436e9: Don't call debug on failed cl-assert

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 689 bytes --]

On 2016-11-08 19:55, Noam Postavsky wrote:
> [...] Meanwhile, I've pushed the (funcall debugger...) solution to 
> emacs-25 since that is less of a change vs 25.1 than what's there 
> currently.

Hi Noam and Stefan,

Given the current implementation, how does one catch a failed assertion in a condition-case block (when debug-on-error is true)?  If there is no easy way, could we add a defvar to make this behavior of cl-assertion-failed (calling the debugger directly) optional?

(The broader context is that I'm trying to log all errors produced by an Emacs server that doesn't have a terminal to write to, and I can't easily catch failed assertions.)

Thanks!
Clément.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Emacs-diffs] emacs-25 db436e9: Don't call debug on failed cl-assert

[Noam Postavsky]

On Fri, Jan 27, 2017 at 8:16 PM, Clément Pit--Claudel
<clement.pit@gmail.com> wrote:
> On 2016-11-08 19:55, Noam Postavsky wrote:
>> [...] Meanwhile, I've pushed the (funcall debugger...) solution to
>> emacs-25 since that is less of a change vs 25.1 than what's there
>> currently.
>
> Hi Noam and Stefan,
>
> Given the current implementation, how does one catch a failed assertion in a condition-case block (when debug-on-error is true)?  If there is no easy way, could we add a defvar to make this behavior of cl-assertion-failed (calling the debugger directly) optional?

I guess let-binding `debugger' to a function which performs the
logging should do the trick?

>
> (The broader context is that I'm trying to log all errors produced by an Emacs server that doesn't have a terminal to write to, and I can't easily catch failed assertions.)
>
> Thanks!
> Clément.
>

Re: [Emacs-diffs] emacs-25 db436e9: Don't call debug on failed cl-assert

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 931 bytes --]

On 2017-01-27 20:59, Noam Postavsky wrote:
> I guess let-binding `debugger' to a function which performs the
> logging should do the trick?

I already do that, in fact, and I do re-throw the exception from there.  Looks like things didn't work because of the way `debugger' is called in `cl--assertion-failed'.  Is that call correct?  The argument in (funcall debugger `(cl-assertion-failed ,form ,string ,@sargs)) doesn't seem to match the docs:

    Documentation:
    Function to call to invoke debugger.
    If due to frame exit, args are ‘exit’ and the value being returned;
     this function’s value will be returned instead of that.
    If due to error, args are ‘error’ and a list of the args to ‘signal’.
    If due to ‘apply’ or ‘funcall’ entry, one arg, ‘lambda’.
    If due to ‘eval’ entry, one arg, t.

Is the call just missing an 'error argument?

Thanks,
Clément.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Emacs-diffs] emacs-25 db436e9: Don't call debug on failed cl-assert

[Noam Postavsky]

On Fri, Jan 27, 2017 at 10:30 PM, Clément Pit--Claudel
<clement.pit@gmail.com> wrote:
> On 2017-01-27 20:59, Noam Postavsky wrote:
>> I guess let-binding `debugger' to a function which performs the
>> logging should do the trick?
>
> I already do that, in fact, and I do re-throw the exception from there.  Looks like things didn't work because of the way `debugger' is called in `cl--assertion-failed'.  Is that call correct?  The argument in (funcall debugger `(cl-assertion-failed ,form ,string ,@sargs)) doesn't seem to match the docs:
>
>     If due to error, args are ‘error’ and a list of the args to ‘signal’.
>
> Is the call just missing an 'error argument?
>

Oh yeah, I guess it is.

Re: [Emacs-diffs] emacs-25 db436e9: Don't call debug on failed cl-assert

[Noam Postavsky]

On Fri, Jan 27, 2017 at 10:46 PM, Noam Postavsky
<npostavs@users.sourceforge.net> wrote:
> On Fri, Jan 27, 2017 at 10:30 PM, Clément Pit--Claudel
> <clement.pit@gmail.com> wrote:
>> On 2017-01-27 20:59, Noam Postavsky wrote:
>>> I guess let-binding `debugger' to a function which performs the
>>> logging should do the trick?
>>
>> I already do that, in fact, and I do re-throw the exception from there.  Looks like things didn't work because of the way `debugger' is called in `cl--assertion-failed'.  Is that call correct?  The argument in (funcall debugger `(cl-assertion-failed ,form ,string ,@sargs)) doesn't seem to match the docs:
>>
>>     If due to error, args are ‘error’ and a list of the args to ‘signal’.
>>
>> Is the call just missing an 'error argument?
>>
>
> Oh yeah, I guess it is.

Um, should this be

(funcall debugger 'error `(cl-assertion-failed ,form ,string ,@sargs))

or

(funcall debugger 'error `(cl-assertion-failed (,form ,string ,@sargs)))

I'm getting confused with all the levels of nesting and funcall/apply.

Re: [Emacs-diffs] emacs-25 db436e9: Don't call debug on failed cl-assert

[Clément Pit-Claudel]

On 2017-01-29 10:37, Noam Postavsky wrote:
> On Fri, Jan 27, 2017 at 10:46 PM, Noam Postavsky
> <npostavs@users.sourceforge.net> wrote:
>> On Fri, Jan 27, 2017 at 10:30 PM, Clément Pit--Claudel
>> <clement.pit@gmail.com> wrote:
>>> On 2017-01-27 20:59, Noam Postavsky wrote:
>>>> I guess let-binding `debugger' to a function which performs the
>>>> logging should do the trick?
>>>
>>> I already do that, in fact, and I do re-throw the exception from there.  Looks like things didn't work because of the way `debugger' is called in `cl--assertion-failed'.  Is that call correct?  The argument in (funcall debugger `(cl-assertion-failed ,form ,string ,@sargs)) doesn't seem to match the docs:
>>>
>>>     If due to error, args are ‘error’ and a list of the args to ‘signal’.
>>>
>>> Is the call just missing an 'error argument?
>>>
>>
>> Oh yeah, I guess it is.
> 
> Um, should this be
> 
> (funcall debugger 'error `(cl-assertion-failed ,form ,string ,@sargs))
> 
> or
> 
> (funcall debugger 'error `(cl-assertion-failed (,form ,string ,@sargs)))
> 
> I'm getting confused with all the levels of nesting and funcall/apply.


I think the second one.  `debugger' gets two arguments, 'error and a list of arguments to `signal'.  That list should have two entries: `cl-assertion-failed' and `(,form ,string ,@sargs).

Thanks! 

Re: [Emacs-diffs] emacs-25 db436e9: Don't call debug on failed cl-assert

[Noam Postavsky]

On Sun, Jan 29, 2017 at 4:09 PM, Clément Pit-Claudel
<cpitclaudel@gmail.com> wrote:
>> Um, should this be
>>
>> (funcall debugger 'error `(cl-assertion-failed ,form ,string ,@sargs))
>>
>> or
>>
>> (funcall debugger 'error `(cl-assertion-failed (,form ,string ,@sargs)))
>>
>> I'm getting confused with all the levels of nesting and funcall/apply.
>
>
> I think the second one.  `debugger' gets two arguments, 'error and a list of arguments to `signal'.  That list should have two entries: `cl-assertion-failed' and `(,form ,string ,@sargs).
>

Pushed to emacs-25 [1: 72ef710].

1: 2017-01-30 21:45:02 -0500 72ef710f6e1c8e334fd50da9480a8cb151e823a2
  Fix call to debugger on assertion failure