bug#30823: 25.3; modification-hooks of overlays are not run in some cases

[Eli Zaretskii <eliz@gnu.org>]

> From: Noam Postavsky <npostavs@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  30823@debbugs.gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Fri, 17 Aug 2018 16:52:54 -0400
> 
> Noam Postavsky <npostavs@gmail.com> writes:
> 
> > @@ -10403,6 +10403,13 @@ message_dolog (const char *m, ptrdiff_t nbytes, bool nlflag, bool multibyte)
> >  	  ptrdiff_t this_bol, this_bol_byte, prev_bol, prev_bol_byte;
> >  	  printmax_t dups;
> >  
> > +          /* Since we call del_range_both passing false for PREPARE,
> > +             we aren't prepared to run modification hooks (we could
> > +             end up calling modification hooks from another buffer and
> > +             only with AFTER=t, Bug#21824).  */
> > +          ptrdiff_t count = SPECPDL_INDEX ();
> > +          specbind (Qinhibit_modification_hooks, Qt);
> > +
> >  	  insert_1_both ("\n", 1, 1, true, false, false);
> >  
> >  	  scan_newline (Z, Z_BYTE, BEG, BEG_BYTE, -2, false);
> 
> Coming back to this, there is also the possibility of passing true for
> PREPARE, though I'm not sure if that would be better or worse.  Any
> comments?

AFAIR, we never want to use PREPARE = true when dealing with the
*Messages* buffer, you can see that elsewhere in message_dolog.  The
reason I believe is that we might trigger infinite recursion if the
modification hooks log a message for some reason.

Btw, I'm somewhat worried by the solution being proposed: it removes a
general safety device and replaces it by a solution that targets only
bug#21824, a much narrower class of problems.  Is that wise?

Can we turn the table and ask whether it makes sense to delete an
overlay from the modification hooks of that same overlay?  Maybe
ggtags needs to find a better/safer solution for whatever feature it
wants to implement?