bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Achim Gratz]

In GNU Emacs 24.2.1 (i686-suse-linux-gnu, GTK+ Version 3.4.4)
 of 2012-09-30 on Rainer

I've just removed some utf-8 codepoints from docstrings in org-mode
because when I compress either the source (.el.gz) or the resulting
byte-compiled file (.elc.gz), the loader fails after the first function
definition that has such a docstring.  Messages suspiciously said
something about "loading with code-conversion", so I assume that the the
coding-system was not correctly recognized and the file mangled as a
result before it reached the loader.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Terratec KOMPLEXER:
http://Synth.Stromeko.net/Downloads.html#KomplexerWaves

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Stefan Monnier]

> I've just removed some utf-8 codepoints from docstrings in org-mode
> because when I compress either the source (.el.gz) or the resulting
> byte-compiled file (.elc.gz), the loader fails after the first function
> definition that has such a docstring.

Sounds like a bug.  Can you send a precise recipe?


        Stefan

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Achim Gratz]

Stefan Monnier writes:
>> I've just removed some utf-8 codepoints from docstrings in org-mode
>> because when I compress either the source (.el.gz) or the resulting
>> byte-compiled file (.elc.gz), the loader fails after the first function
>> definition that has such a docstring.
>
> Sounds like a bug.  Can you send a precise recipe?

The org.el currently in trunk should demonstrate it: compress the
bytecompiled file with gzip, then try to (load "org") with the load-path
set so that it finds those compressed files.  With the byte-compiled
file you'll then get this error:

Debugger entered--Lisp error: (void-variable defalias)
  eval-buffer(#<buffer  *load*> nil "/home/emacs/lisp/org/org.elc.gz" nil t)  ; Reading at buffer position 317443
  load-with-code-conversion("/home/emacs/lisp/org/org.elc.gz" "/home/emacs/lisp/org/org.elc.gz" nil nil)
  load("org.elc")
  eval((load "org.elc"))
  eval-expression((load "org.elc") nil)
  call-interactively(eval-expression nil nil)

The corresponding line in the source is L9026 (the defun for the second
function that has unicode in the docstring).

The exact same error happens in Emacs 23.3, btw.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for Waldorf Q V3.00R3 and Q+ V3.54R2:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Andreas Schwab]

Achim Gratz <Stromeko@nexgo.de> writes:

> Stefan Monnier writes:
>>> I've just removed some utf-8 codepoints from docstrings in org-mode
>>> because when I compress either the source (.el.gz) or the resulting
>>> byte-compiled file (.elc.gz), the loader fails after the first function
>>> definition that has such a docstring.
>>
>> Sounds like a bug.  Can you send a precise recipe?
>
> The org.el currently in trunk should demonstrate it: compress the
> bytecompiled file with gzip, then try to (load "org") with the load-path
> set so that it finds those compressed files.  With the byte-compiled
> file you'll then get this error:
>
> Debugger entered--Lisp error: (void-variable defalias)

That's because #@N counts in bytes, not characters.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Achim Gratz]

Andreas Schwab writes:
>> Debugger entered--Lisp error: (void-variable defalias)
>
> That's because #@N counts in bytes, not characters.

I know, that's why I used goto-char to locate the place in org.elc and
then the correspoding definition in org.el.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Achim Gratz]

Achim Gratz writes:
> Andreas Schwab writes:
>>> Debugger entered--Lisp error: (void-variable defalias)
>>
>> That's because #@N counts in bytes, not characters.
>
> I know, that's why I used goto-char to locate the place in org.elc and
> then the correspoding definition in org.el.

I've just decoded what you were telling me with that one-liner.
Disregard what I wrote above, not enough coffee yet…

So, any codepoint that is more than a single byte will throw the
byte-compiler off, not just any utf-8 codepoint.  Since this has been in
Emacs likely ever since unicode strings have been introduced, I'd
suggest adding a *strong* warning in some prominent place in the
documentation about this even when it gets fixed in a newer version of
Emacs. Otherwise it's all too easy to produce libraries that have
mysterious failures depending on whatever Emacs was used to compile or
run them.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf Blofeld V1.15B11:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Stefan Monnier]

> I've just removed some utf-8 codepoints from docstrings in org-mode
> because when I compress either the source (.el.gz) or the resulting
> byte-compiled file (.elc.gz), the loader fails after the first function

I can't reproduce this problem for the .el.gz case (indeed, I think
it's specific to byte-compiled files).

> So, any codepoint that is more than a single byte will throw the
> byte-compiler off, not just any utf-8 codepoint.  Since this has been in
> Emacs likely ever since unicode strings have been introduced, I'd
> suggest adding a *strong* warning in some prominent place in the
> documentation about this even when it gets fixed in a newer version of
> Emacs. Otherwise it's all too easy to produce libraries that have
> mysterious failures depending on whatever Emacs was used to compile or
> run them.

I think the problem lies between load-with-code-conversion and
eval-buffer, so it dates back to the introduction of
load-with-code-conversion, which IIRC predates the internal use
of Unicode.

Fixing `eval-buffer' so that it skips bytes when it sees #@NN is tricky,
so the best fix is probably to change load-with-code-conversion so that
(if the file is byte-compiled) it saves the buffer to a temp file and
passes that to `load'.


        Stefan

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Achim Gratz]

Stefan Monnier writes:
> I can't reproduce this problem for the .el.gz case (indeed, I think
> it's specific to byte-compiled files).

I would have to try and re-create the problem, but it may be the case
that the problem only affects the byte-compiled files.

[…]
> Fixing `eval-buffer' so that it skips bytes when it sees #@NN is tricky,
> so the best fix is probably to change load-with-code-conversion so that
> (if the file is byte-compiled) it saves the buffer to a temp file and
> passes that to `load'.

I'm not sure I can follow you, especially what the purpose of saving to
a temporary file is.  As I said, I haven't looked at this for quite some
time.  If you think it helps, I'll try to set up for reproducing this
again.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for KORG EX-800 and Poly-800MkII V0.9:
http://Synth.Stromeko.net/Downloads.html#KorgSDada

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Kenichi Handa]

In article <jwvmwvpgrcg.fsf-monnier+bug#12598@gnu.org>, Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> I think the problem lies between load-with-code-conversion and
> eval-buffer, so it dates back to the introduction of
> load-with-code-conversion, which IIRC predates the internal use
> of Unicode.

> Fixing `eval-buffer' so that it skips bytes when it sees #@NN is tricky,
> so the best fix is probably to change load-with-code-conversion so that
> (if the file is byte-compiled) it saves the buffer to a temp file and
> passes that to `load'.

The variable load-source-file-function is set to
load-with-code-converision, and Fload calls
load-source-file-function.  So, shouldn't we provide a file
name handler for `load' operation instead of
making load-source-file-function handle non-source files?

---
Kenichi Handa
handa@gnu.org

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Stefan Monnier]

>> Fixing `eval-buffer' so that it skips bytes when it sees #@NN is tricky,
>> so the best fix is probably to change load-with-code-conversion so that
>> (if the file is byte-compiled) it saves the buffer to a temp file and
>> passes that to `load'.
> The variable load-source-file-function is set to
> load-with-code-converision, and Fload calls
> load-source-file-function.  So, shouldn't we provide a file
> name handler for `load' operation instead of
> making load-source-file-function handle non-source files?

We could, but I suspect that the problem (and the fix) would be the same
for all file-name-handlers.  IOW, the problem is not in how to access
the file's contents, but in how eval-buffer (used by
load-with-code-conversion) doesn't handle byte-compiled file correctly.


        Stefan

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Kenichi Handa]

In article <jwv4nhwrv0k.fsf-monnier+emacs@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca> writes:

> We could, but I suspect that the problem (and the fix) would be the same
> for all file-name-handlers.  IOW, the problem is not in how to access
> the file's contents, but in how eval-buffer (used by
> load-with-code-conversion) doesn't handle byte-compiled file correctly.

Should eval-buffer handle byte-compiled code in a buffer?

---
Kenichi Handa
handa@gnu.org

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Stefan Monnier]

>> We could, but I suspect that the problem (and the fix) would be the same
>> for all file-name-handlers.  IOW, the problem is not in how to access
>> the file's contents, but in how eval-buffer (used by
>> load-with-code-conversion) doesn't handle byte-compiled file correctly.
> Should eval-buffer handle byte-compiled code in a buffer?

That'd be nice, tho it might be kind of annoying to implement (having to
look at buffer-file-coding-system to figure out the byte-size of each
char when skipping #@NN).


        Stefan

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Stefan Monnier]

>> We could, but I suspect that the problem (and the fix) would be the same
>> for all file-name-handlers.  IOW, the problem is not in how to access
>> the file's contents, but in how eval-buffer (used by
>> load-with-code-conversion) doesn't handle byte-compiled file correctly.
> Should eval-buffer handle byte-compiled code in a buffer?

BTW, making it work might be nice, but it's only worth the trouble if
the result is correct.  I.e. after correctly skipping the #@NNN, we also
want to make sure that subsequent needs to load the corresponding
dynamic docstring or (or dynamic bytecode) can correctly find it.


        Stefan

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Kenichi Handa]

In article <jwv7gmpl72g.fsf-monnier+emacs@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca> writes:

> > Should eval-buffer handle byte-compiled code in a buffer?

> That'd be nice, tho it might be kind of annoying to implement (having to
> look at buffer-file-coding-system to figure out the byte-size of each
> char when skipping #@NN).

If we don't change eval-buffer, we should fix Fload itself.
It has this code now:

  if (!memcmp (SDATA (found) + SBYTES (found) - 4, ".elc", 4)
      || (fd >= 0 && (version = safe_to_load_version (fd)) > 0))
    /* Load .elc files directly, but not when they are
       remote and have no handler!  */
[...]
  else
    {
      /* We are loading a source file (*.el).  */
      if (!NILP (Vload_source_file_function))
	{
	  Lisp_Object val;

	  if (fd >= 0)
	    emacs_close (fd);
	  val = call4 (Vload_source_file_function, found, hist_file_name,
		       NILP (noerror) ? Qnil : Qt,
		       (NILP (nomessage) || force_load_messages) ? Qnil : Qt);
	  return unbind_to (count, val);
	}
    }

Apparently, Fload doesn't intend to handle a compressed
byte-compiled file.  And, it is possible to bind
load-source-file-function to some other function than
load-with-code-conversion, modifing
load-with-code-conversion is not enough.

---
Kenichi Handa
handa@gnu.org

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Stefan Monnier]

> Apparently, Fload doesn't intend to handle a compressed
> byte-compiled file.  And, it is possible to bind
> load-source-file-function to some other function than
> load-with-code-conversion, modifing
> load-with-code-conversion is not enough.

I don't know of any case where load-source-file-function is rebound to
something else, so it's hard to judge.


        Stefan

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Kenichi Handa]

In article <jwv6226hd6l.fsf-monnier+emacs@gnu.org>, Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> > Apparently, Fload doesn't intend to handle a compressed
> > byte-compiled file.  And, it is possible to bind
> > load-source-file-function to some other function than
> > load-with-code-conversion, modifing
> > load-with-code-conversion is not enough.

> I don't know of any case where load-source-file-function is rebound to
> something else, so it's hard to judge.

I don't know the concrete example either.  But, at least the
name and the docstring of load-source-file-function implies
that the function bound to it doesn't have to handle
compressed byte-compiled file.

---
Kenichi Handa
handa@gnu.org

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Stefan Monnier]

> Debugger entered--Lisp error: (void-variable defalias)
>   eval-buffer(#<buffer  *load*> nil "/home/emacs/lisp/org/org.elc.gz" nil
> t)  ; Reading at buffer position 317443
>   load-with-code-conversion("/home/emacs/lisp/org/org.elc.gz"
> "/home/emacs/lisp/org/org.elc.gz" nil nil)
>   load("org.elc")
>   eval((load "org.elc"))
>   eval-expression((load "org.elc") nil)
>   call-interactively(eval-expression nil nil)

I notice here that jka-compr-load is supposed to handle this case, but
was somehow not triggered.  Maybe we should fix that part.


        Stefan

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Stefan Monnier]

> I've just removed some utf-8 codepoints from docstrings in org-mode
> because when I compress either the source (.el.gz) or the resulting
> byte-compiled file (.elc.gz), the loader fails after the first function
> definition that has such a docstring.  Messages suspiciously said
> something about "loading with code-conversion", so I assume that the the
> coding-system was not correctly recognized and the file mangled as a
> result before it reached the loader.

I've installed a workaround in the trunk, which should fix eval-buffer
such that the above now works.
I'm not sure if after loading the file, the docstrings will be correctly
found, but at least, loading the file should work.


        Stefan

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Kenichi Handa]

In article <jwvwqulebdh.fsf-monnier+bug#12598@gnu.org>, Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> I notice here that jka-compr-load is supposed to handle this case, but
> was somehow not triggered.  Maybe we should fix that part.

It seems that Fload explicitly suppresses that trigger.

  /* If file name is magic, call the handler.  */
  /* This shouldn't be necessary any more now that `openp' handles it right.
    handler = Ffind_file_name_handler (file, Qload);
    if (!NILP (handler))
      return call5 (handler, Qload, file, noerror, nomessage, nosuffix); */

I don't understand the above comment (why can openp handle
this case?), and I confirmed that loading *.elc.gz works
well by enabling this code again.

---
Kenichi Handa
handa@gnu.org

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Eli Zaretskii]

> From: Kenichi Handa <handa@gnu.org>
> Date: Sat, 09 Feb 2013 14:05:50 +0900
> Cc: Stromeko@nexgo.de, 12598@debbugs.gnu.org
> 
> In article <jwvwqulebdh.fsf-monnier+bug#12598@gnu.org>, Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
> 
> > I notice here that jka-compr-load is supposed to handle this case, but
> > was somehow not triggered.  Maybe we should fix that part.
> 
> It seems that Fload explicitly suppresses that trigger.
> 
>   /* If file name is magic, call the handler.  */
>   /* This shouldn't be necessary any more now that `openp' handles it right.
>     handler = Ffind_file_name_handler (file, Qload);
>     if (!NILP (handler))
>       return call5 (handler, Qload, file, noerror, nomessage, nosuffix); */
> 
> I don't understand the above comment (why can openp handle
> this case?)

See "bzr diff -c 39793".  In that revision, the SUFFIXES argument to
openp was changed, and its handling inside openp was changed as well.
I believe the intent was to let openp call the file handler.

> and I confirmed that loading *.elc.gz works well by enabling this
> code again.

Why doesn't it work when openp calls the same handler?

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Kenichi Handa]

In article <83pq097sp3.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:

> See "bzr diff -c 39793".  In that revision, the SUFFIXES argument to
> openp was changed, and its handling inside openp was changed as well.
> I believe the intent was to let openp call the file handler.

But, openp just checks whether or not any handlers exist, it
doesn't call a found handler.

---
Kenichi Handa
handa@gnu.org

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Stefan Monnier]

>> See "bzr diff -c 39793".  In that revision, the SUFFIXES argument to
>> openp was changed, and its handling inside openp was changed as well.
>> I believe the intent was to let openp call the file handler.
> But, openp just checks whether or not any handlers exist, it
> doesn't call a found handler.

So re-enabling the

  handler = Ffind_file_name_handler (file, Qload);

code might be the best fix.  But does it really work?  I mean, with
byte-compile-dynamic file for example (or to find the docstrings after
loading the file)?


        Stefan

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Lars Ingebrigtsen]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> So re-enabling the
>
>   handler = Ffind_file_name_handler (file, Qload);
>
> code might be the best fix.  But does it really work?  I mean, with
> byte-compile-dynamic file for example (or to find the docstrings after
> loading the file)?

I've added a failing test case to Emacs 29.  If we re-enable that bit in
Fload, the test no longer fails (but there may be other test cases; feel
free to amend).

However, this makes files-tests-file-name-non-special-load fail --
perhaps Michael has some insights here; added to the CCs.

But the logic of Fload is pretty unclear.  It seems we do end up calling
find-file-name-handler anyway, but later:

  /* If FD is -2, that means openp found a magic file.  */
  if (fd == -2)
    {
      if (NILP (Fequal (found, file)))
	/* If FOUND is a different file name from FILE,
	   find its handler even if we have already inhibited
	   the `load' operation on FILE.  */
	handler = Ffind_file_name_handler (found, Qt);
      else
	handler = Ffind_file_name_handler (found, Qload);
      if (! NILP (handler))
	return call5 (handler, Qload, found, noerror, nomessage, Qt);

Hm...  is this just because of the way that test is set up?  

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Michael Albinus]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

Hi Lars,

> I've added a failing test case to Emacs 29.  If we re-enable that bit in
> Fload, the test no longer fails (but there may be other test cases; feel
> free to amend).
>
> However, this makes files-tests-file-name-non-special-load fail --
> perhaps Michael has some insights here; added to the CCs.

When you enable the file name handler in Fload, you must adapt the
respective test like all other files-tests-file-name-non-special-* tests
(currently, is is expecting that the file name handler is not called in Fload).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 633 bytes --]

diff --git a/test/lisp/files-tests.el b/test/lisp/files-tests.el
index 7d17fbde67..ff717d6234 100644
--- a/test/lisp/files-tests.el
+++ b/test/lisp/files-tests.el
@@ -931,7 +931,7 @@ files-tests-file-name-non-special-load
   (files-tests--with-temp-non-special (tmpfile nospecial)
     (should (load nospecial nil t)))
   (files-tests--with-temp-non-special-and-file-name-handler (tmpfile nospecial)
-    (should (load nospecial nil t))))
+    (should-error (load nospecial nil t))))

 (ert-deftest files-tests-file-name-non-special-make-auto-save-file-name ()
   (files-tests--with-temp-non-special (tmpfile nospecial)

[-- Attachment #3: Type: text/plain, Size: 24 bytes --]


Best regards, Michael.

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Lars Ingebrigtsen]

Michael Albinus <michael.albinus@gmx.de> writes:

> When you enable the file name handler in Fload, you must adapt the
> respective test like all other files-tests-file-name-non-special-* tests
> (currently, is is expecting that the file name handler is not called in Fload).

Oh, was that was it was testing -- I didn't understand the logic there.

After adjusting that test, Emacs seems to work fine, and it fixes the
originally reported problems.  Does anybody else see any problems with
reinstating the file name handler in Fload?  

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#12598: 24.2; utf-8 codepoints in doc-strings and compression of .el and .elc files

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> After adjusting that test, Emacs seems to work fine, and it fixes the
> originally reported problems.  Does anybody else see any problems with
> reinstating the file name handler in Fload?  

Nobody had any further comments, so I've now reinstated that file name
handler.  Give a yell if this broke anything -- tests pass fine, but...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no