* bug#26058: utf16->string and utf32->string don't conform to R6RS @ 2017-03-11 12:19 "Taylan Ulrich Bayırlı/Kammer" 2017-03-13 13:03 ` Andy Wingo 0 siblings, 1 reply; 8+ messages in thread From: "Taylan Ulrich Bayırlı/Kammer" @ 2017-03-11 12:19 UTC (permalink / raw) To: 26058 See the R6RS Libraries document page 10. The differences: - R6RS supports reading a BOM. - R6RS mandates an endianness argument to specify the behavior at the absence of a BOM. - R6RS allows an optional third argument 'endianness-mandatory' to explicitly ignore any possible BOM. Here's a quick patch on top of master. I didn't test it thoroughly... ===File /home/taylan/src/guile/guile-master/0001-Fix-R6RS-utf16-string-and-utf32-string.patch=== From f51cd1d4884caafb1ed0072cd77c0e3145f34576 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Taylan=20Ulrich=20Bay=C4=B1rl=C4=B1/Kammer?= <taylanbayirli@gmail.com> Date: Fri, 10 Mar 2017 22:36:55 +0100 Subject: [PATCH] Fix R6RS utf16->string and utf32->string. * module/rnrs/bytevectors.scm (read-bom16, read-bom32): New procedures. (r6rs-utf16->string, r6rs-utf32->string): Ditto. --- module/rnrs/bytevectors.scm | 52 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/module/rnrs/bytevectors.scm b/module/rnrs/bytevectors.scm index 9744359f0..997a8c9cb 100644 --- a/module/rnrs/bytevectors.scm +++ b/module/rnrs/bytevectors.scm @@ -69,7 +69,9 @@ bytevector-ieee-double-native-set! string->utf8 string->utf16 string->utf32 - utf8->string utf16->string utf32->string)) + utf8->string + (r6rs-utf16->string . utf16->string) + (r6rs-utf32->string . utf32->string))) (load-extension (string-append "libguile-" (effective-version)) @@ -80,4 +82,52 @@ `(quote ,sym) (error "unsupported endianness" sym))) +(define (read-bom16 bv) + (let ((c0 (bytevector-u8-ref bv 0)) + (c1 (bytevector-u8-ref bv 1))) + (cond + ((and (= c0 #xFE) (= c1 #xFF)) + 'big) + ((and (= c0 #xFF) (= c1 #xFE)) + 'little) + (else + #f)))) + +(define r6rs-utf16->string + (case-lambda + ((bv default-endianness) + (let ((bom-endianness (read-bom16 bv))) + (if (not bom-endianness) + (utf16->string bv default-endianness) + (substring/shared (utf16->string bv bom-endianness) 1)))) + ((bv endianness endianness-mandatory?) + (if endianness-mandatory? + (utf16->string bv endianness) + (r6rs-utf16->string bv endianness))))) + +(define (read-bom32 bv) + (let ((c0 (bytevector-u8-ref bv 0)) + (c1 (bytevector-u8-ref bv 1)) + (c2 (bytevector-u8-ref bv 2)) + (c3 (bytevector-u8-ref bv 3))) + (cond + ((and (= c0 #x00) (= c1 #x00) (= c2 #xFE) (= c3 #xFF)) + 'big) + ((and (= c0 #xFF) (= c1 #xFE) (= c2 #x00) (= c3 #x00)) + 'little) + (else + #f)))) + +(define r6rs-utf32->string + (case-lambda + ((bv default-endianness) + (let ((bom-endianness (read-bom32 bv))) + (if (not bom-endianness) + (utf32->string bv default-endianness) + (substring/shared (utf32->string bv bom-endianness) 1)))) + ((bv endianness endianness-mandatory?) + (if endianness-mandatory? + (utf32->string bv endianness) + (r6rs-utf32->string bv endianness))))) + ;;; bytevector.scm ends here -- 2.11.0 ============================================================ ^ permalink raw reply related [flat|nested] 8+ messages in thread
* bug#26058: utf16->string and utf32->string don't conform to R6RS 2017-03-11 12:19 bug#26058: utf16->string and utf32->string don't conform to R6RS "Taylan Ulrich Bayırlı/Kammer" @ 2017-03-13 13:03 ` Andy Wingo 2017-03-13 18:10 ` Taylan Ulrich Bayırlı/Kammer 0 siblings, 1 reply; 8+ messages in thread From: Andy Wingo @ 2017-03-13 13:03 UTC (permalink / raw) To: "Taylan Ulrich "Bayırlı/Kammer""; +Cc: 26058 On Sat 11 Mar 2017 13:19, taylanbayirli@gmail.com ("Taylan Ulrich "Bayırlı/Kammer"") writes: > See the R6RS Libraries document page 10. The differences: > > - R6RS supports reading a BOM. > > - R6RS mandates an endianness argument to specify the behavior at the > absence of a BOM. > > - R6RS allows an optional third argument 'endianness-mandatory' to > explicitly ignore any possible BOM. > > Here's a quick patch on top of master. I didn't test it thoroughly... Hi, this is a tricky area that is not so amenable to quick patches :) Have you looked into what Guile already does for byte-order marks? Can you explain how the R6RS specification relates to this? https://www.gnu.org/software/guile/manual/html_node/BOM-Handling.html Andy ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#26058: utf16->string and utf32->string don't conform to R6RS 2017-03-13 13:03 ` Andy Wingo @ 2017-03-13 18:10 ` Taylan Ulrich Bayırlı/Kammer 2017-03-13 21:24 ` Andy Wingo 0 siblings, 1 reply; 8+ messages in thread From: Taylan Ulrich Bayırlı/Kammer @ 2017-03-13 18:10 UTC (permalink / raw) To: Andy Wingo; +Cc: 26058 Andy Wingo <wingo@pobox.com> writes: > Hi, > > this is a tricky area that is not so amenable to quick patches :) Have > you looked into what Guile already does for byte-order marks? Can you > explain how the R6RS specification relates to this? > > https://www.gnu.org/software/guile/manual/html_node/BOM-Handling.html > > Andy Hmm, interesting. I noticed the utf{16,32}->string procedures ignoring a BOM at the start of the given bytevector, but didn't look at it from a ports perspective. TL;DR of the below though: the R6RS semantics offer a strict enrichment of the feature-set of the utfX->string procedures relative to the Guile semantics, so at most we would end up with spurious features. (The optional ability to handle any possible BOM at the start of the bytevector, with a fall-back endianness in case none is found.) That said, let's see... If I do a textual read from a port, I already get a string and not a bytevector, so the behavior of utfX->string operations is irrelevant. If I do binary I/O, the following situations are possible: 1. I'm guaranteed to get any possible bytes that happen to form a valid BOM at the start of the stream as-is in the returned bytevector; the binary I/O interface doesn't see such bytes as anything special, as it could simply be coincidence that the stream starts with such bytes. 2. I'm guaranteed *not* to get bytes that form a BOM at the start of the stream; instead they're consumed to set the port encoding for any future text I/O. 3. The behavior is unspecified and either of the above may happen. In the case of #1, it's probably good for utfX->string procedures to be able to handle BOMs, but also allow explicitly ignoring any possible BOM. The R6RS semantics cover this. In the case of #2, the utfX->string procedures don't need to be able to handle BOMs as far as we're talking about passing them bytevectors returned by port I/O, but it also doesn't hurt if they optionally support it. The R6RS semantics are fine here as well I think. As for #3... first of all it's bad IMO; the behavior ought to be specified. :-) But in any case, the additional features of the R6RS semantics won't hurt. WDYT? As far as I understand the page you linked, Guile currently implements #3, which I think is unfortunate but can kinda understand too. In any case, the additional R6RS features won't hurt, right? Taylan ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#26058: utf16->string and utf32->string don't conform to R6RS 2017-03-13 18:10 ` Taylan Ulrich Bayırlı/Kammer @ 2017-03-13 21:24 ` Andy Wingo 2017-03-14 15:03 ` Taylan Ulrich Bayırlı/Kammer 0 siblings, 1 reply; 8+ messages in thread From: Andy Wingo @ 2017-03-13 21:24 UTC (permalink / raw) To: Taylan Ulrich "Bayırlı/Kammer"; +Cc: 26058 On Mon 13 Mar 2017 19:10, taylanbayirli@gmail.com (Taylan Ulrich "Bayırlı/Kammer") writes: > If I do binary I/O, the following situations are possible: > > 1. I'm guaranteed to get any possible bytes that happen to form a valid > BOM at the start of the stream as-is in the returned bytevector; the > binary I/O interface doesn't see such bytes as anything special, as > it could simply be coincidence that the stream starts with such > bytes. > > 2. I'm guaranteed *not* to get bytes that form a BOM at the start of the > stream; instead they're consumed to set the port encoding for any > future text I/O. > > 3. The behavior is unspecified and either of the above may happen. (1). But I thought this bug was about using a bytevector as a source and then doing textual I/O on it, no? Andy ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#26058: utf16->string and utf32->string don't conform to R6RS 2017-03-13 21:24 ` Andy Wingo @ 2017-03-14 15:03 ` Taylan Ulrich Bayırlı/Kammer 2017-03-14 15:44 ` Andy Wingo 0 siblings, 1 reply; 8+ messages in thread From: Taylan Ulrich Bayırlı/Kammer @ 2017-03-14 15:03 UTC (permalink / raw) To: Andy Wingo; +Cc: 26058 Andy Wingo <wingo@pobox.com> writes: > On Mon 13 Mar 2017 19:10, taylanbayirli@gmail.com (Taylan Ulrich "Bayırlı/Kammer") writes: > >> If I do binary I/O, the following situations are possible: >> >> 1. I'm guaranteed to get any possible bytes that happen to form a valid >> BOM at the start of the stream as-is in the returned bytevector; the >> binary I/O interface doesn't see such bytes as anything special, as >> it could simply be coincidence that the stream starts with such >> bytes. > > (1). But I thought this bug was about using a bytevector as a source > and then doing textual I/O on it, no? I have a feeling we're somehow talking past each other. :-) As far as I'm concerned, the bug is just that the procedures don't conform to the specification. It would of course be good if the behavior of these procedures was somehow in harmony with the behavior of I/O operations, but I don't see any issues arising from adopting the R6RS behavior of the procedures utf16->string and utf32->string. Do you? Taylan ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#26058: utf16->string and utf32->string don't conform to R6RS 2017-03-14 15:03 ` Taylan Ulrich Bayırlı/Kammer @ 2017-03-14 15:44 ` Andy Wingo 2017-03-16 19:34 ` Taylan Ulrich Bayırlı/Kammer 0 siblings, 1 reply; 8+ messages in thread From: Andy Wingo @ 2017-03-14 15:44 UTC (permalink / raw) To: Taylan Ulrich "Bayırlı/Kammer"; +Cc: 26058 On Tue 14 Mar 2017 16:03, taylanbayirli@gmail.com (Taylan Ulrich "Bayırlı/Kammer") writes: > Andy Wingo <wingo@pobox.com> writes: > >> On Mon 13 Mar 2017 19:10, taylanbayirli@gmail.com (Taylan Ulrich "Bayırlı/Kammer") writes: >> >>> If I do binary I/O, the following situations are possible: >>> >>> 1. I'm guaranteed to get any possible bytes that happen to form a valid >>> BOM at the start of the stream as-is in the returned bytevector; the >>> binary I/O interface doesn't see such bytes as anything special, as >>> it could simply be coincidence that the stream starts with such >>> bytes. >> >> (1). But I thought this bug was about using a bytevector as a source >> and then doing textual I/O on it, no? > > I have a feeling we're somehow talking past each other. :-) As far as > I'm concerned, the bug is just that the procedures don't conform to the > specification. > > It would of course be good if the behavior of these procedures was > somehow in harmony with the behavior of I/O operations, but I don't see > any issues arising from adopting the R6RS behavior of the procedures > utf16->string and utf32->string. Do you? Adopting the behavior is more or less fine. If it can be done while relying on the existing behavior, that is better than something ad-hoc in a module. Andy ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#26058: utf16->string and utf32->string don't conform to R6RS 2017-03-14 15:44 ` Andy Wingo @ 2017-03-16 19:34 ` Taylan Ulrich Bayırlı/Kammer 2018-10-15 4:57 ` Mark H Weaver 0 siblings, 1 reply; 8+ messages in thread From: Taylan Ulrich Bayırlı/Kammer @ 2017-03-16 19:34 UTC (permalink / raw) To: Andy Wingo; +Cc: 26058 Andy Wingo <wingo@pobox.com> writes: > Adopting the behavior is more or less fine. If it can be done while > relying on the existing behavior, that is better than something ad-hoc > in a module. You mean somehow leveraging the existing BOM handling code of Guile (found in the ports code) would be preferable to reimplementing it like in this patch, correct? In that light, I had this attempt: (define r6rs-utf16->string (case-lambda ((bv default-endianness) (let* ((binary-port (open-bytevector-input-port bv)) (transcoder (make-transcoder (utf-16-codec))) (utf16-port (transcoded-port binary-port transcoder))) ;; XXX how to set default-endianness for a port? (get-string-all utf16-port))) ((bv endianness endianness-mandatory?) (if endianness-mandatory? (utf16->string bv endianness) (r6rs-utf16->string bv endianness))))) As commented in the first branch of the case-lambda, this does not yet make use of the 'default-endianness' parameter to tell the port transcoder (or whoever) what to do in case no BOM is found in the stream. From what I can tell, Guile is currently hardcoded to *transparently* default to big-endian in ports.c, port_clear_stream_start_for_bom_read. Is there a way to detect when Guile was unable to find a BOM? (In that case one could set the endianness explicitly to the desired default.) Or do you see another way to implement this? Thanks for the feedback! Taylan P.S.: Huge congrats on the big release. :-) ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#26058: utf16->string and utf32->string don't conform to R6RS 2017-03-16 19:34 ` Taylan Ulrich Bayırlı/Kammer @ 2018-10-15 4:57 ` Mark H Weaver 0 siblings, 0 replies; 8+ messages in thread From: Mark H Weaver @ 2018-10-15 4:57 UTC (permalink / raw) To: Taylan Ulrich "Bayırlı/Kammer"; +Cc: 26058 Hi Taylan, taylanbayirli@gmail.com (Taylan Ulrich "Bayırlı/Kammer") writes: > Andy Wingo <wingo@pobox.com> writes: > >> Adopting the behavior is more or less fine. If it can be done while >> relying on the existing behavior, that is better than something ad-hoc >> in a module. In general, I agree with Andy's sentiment that it would be better to avoid redundant BOM handling code, and moreover, I appreciate his reluctance to apply a fix without careful consideration of our existing BOM semantics. However, as Taylan discovered, Guile does not provide a mechanism to specify a default endianness of a UTF-16 or UTF-32 port in case a BOM is not found. I see no straightforward way to implement these R6RS interfaces using ports. We could certainly add such a mechanism if needed, but I see another problem with this approach: the expense of creating and later collecting a bytevector port object would be a very heavy burden to place on these otherwise fairly lightweight operations. Therefore, I would prefer to avoid that implementation strategy for these operations. Although BOM handling for ports is quite complex with many subtle points to consider, detecting a BOM at the beginning of a bytevector is so trivial that I personally have no objection to this tiny duplication of logic. Therefore, my preference would be to adopt code similar to that proposed by Taylan, although I believe it can, and should, be further simplified: > diff --git a/module/rnrs/bytevectors.scm b/module/rnrs/bytevectors.scm > index 9744359f0..997a8c9cb 100644 > --- a/module/rnrs/bytevectors.scm > +++ b/module/rnrs/bytevectors.scm > @@ -69,7 +69,9 @@ > bytevector-ieee-double-native-set! > > string->utf8 string->utf16 string->utf32 > - utf8->string utf16->string utf32->string)) > + utf8->string > + (r6rs-utf16->string . utf16->string) > + (r6rs-utf32->string . utf32->string))) > > > (load-extension (string-append "libguile-" (effective-version)) > @@ -80,4 +82,52 @@ > `(quote ,sym) > (error "unsupported endianness" sym))) > > +(define (read-bom16 bv) > + (let ((c0 (bytevector-u8-ref bv 0)) > + (c1 (bytevector-u8-ref bv 1))) > + (cond > + ((and (= c0 #xFE) (= c1 #xFF)) > + 'big) > + ((and (= c0 #xFF) (= c1 #xFE)) > + 'little) > + (else > + #f)))) We should gracefully handle the case of an empty bytevector, returning an empty string without error in that case. Also, we should use a single 'bytevector-u16-ref' operation to check for the BOM. Pick an arbitrary endianness for the operation (big-endian?), and compare the resulting integer with both #xFEFF and #xFFFE. That way, the code will be simpler and more efficient. Note that our VM has dedicated instructions for these multi-byte bytevector accessors, and there will be fewer comparison operations as well. Similarly for the utf32 case. What do you think? > +(define r6rs-utf16->string > + (case-lambda > + ((bv default-endianness) > + (let ((bom-endianness (read-bom16 bv))) > + (if (not bom-endianness) > + (utf16->string bv default-endianness) > + (substring/shared (utf16->string bv bom-endianness) 1)))) Better to use plain 'substring' here, I think. The machinery of shared substrings is more expensive, and unnecessary in this case. Otherwise, it looks good to me. Would you like to propose a revised patch? Andy, what do you think? Mark ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-10-15 4:57 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-03-11 12:19 bug#26058: utf16->string and utf32->string don't conform to R6RS "Taylan Ulrich Bayırlı/Kammer" 2017-03-13 13:03 ` Andy Wingo 2017-03-13 18:10 ` Taylan Ulrich Bayırlı/Kammer 2017-03-13 21:24 ` Andy Wingo 2017-03-14 15:03 ` Taylan Ulrich Bayırlı/Kammer 2017-03-14 15:44 ` Andy Wingo 2017-03-16 19:34 ` Taylan Ulrich Bayırlı/Kammer 2018-10-15 4:57 ` Mark H Weaver
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).