Re: Lisp watchpoints

[Noam Postavsky]

On Sun, Nov 29, 2015 at 11:43 AM, Eli Zaretskii <address@hidden> wrote:
>> +DEFUN ("remove-variable-watcher", Fremove_variable_watcher, 
>> Sremove_variable_watcher,
>> +       2, 2, 0,
>> +       doc: /* Cause WATCH-FUNCTION to be called when SYMBOL is set. */)
>                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Copy/paste mistake?

Oops.

>
> The call to ARRAYELTS should be outside of the loop (for those poor
> souls who run unoptimized builds most of the time).

Compiling the following shows gcc folds the constants even with -O0
(also, notify_variable_watchers() should only be called for a few
selected symbols so I don't think it would really slow things down
very much).

    int array[] = { 1, 2, 3, 4 };

    /* Number of elements in an array.  */
    #define ARRAYELTS(arr) (sizeof (arr) / sizeof (arr)[0])

    int foo(int x)
    {
        if (x < ARRAYELTS(array))
            return array[x];
        return 0;
    }


>
>>  static const WATCHER_FUNCTION watcher_table[] =
>>    {
>> +    &set_redisplay
>>    };
>>  enum
>>    {
>> +    WATCHER_NUMBER_SET_REDISPLAY
>>    };
>
> Shouldn't we make sure WATCHER_NUMBER_SET_REDISPLAY's value is zero?

Probably better to be explicit, yes.

>
>> +  DEFVAR_INT ("set-redisplay-internal-watcher-number",
>> +              Vset_redisplay_internal_watcher_number,
>> +              doc: /* Internal watch function constant.  */);
>> +  Vset_redisplay_internal_watcher_number = WATCHER_NUMBER_SET_REDISPLAY;
>> +  make_symbol_constant (intern_c_string 
>> ("set-redisplay-internal-watcher-number"));
>
> I'd prefer if all this were moved to window.c.  data.c has no business
> with display-related issues.

Hmm, then how should we make the connection between watcher numbers
and C function pointers? I think data.c should be in charge of that.

>
> Please also add a short notice for etc/NEWS.  Bonus points for adding
> some tests.

Will do.