bug#13485: wrong warning for format ~!

bug#13485: wrong warning for format ~!

[Daniel Llorens]

In 2.0.7

scheme@(guile-user)> (import (ice-9 format))
scheme@(guile-user)> (format #t "~!")
;;; <stdin>:2:0: warning: "~!": wrong number of `format' arguments: expected 1, got 0
$1 = #t
scheme@(guile-user)> (format #t "~!" 3)
$2 = #t
scheme@(guile-user)> (format #t "~!~a" 3 9)  
3$3 = #t
scheme@(guile-user)> (format #t "~a~!" 3 9)  
3$4 = #t
scheme@(guile-user)> (version)
$5 = "2.0.7.28-581f4"

So it doesn't take an argument, it only affects the warning.

Regards,

	Daniel

bug#13485: wrong warning for format ~!

[Ian Price]

Daniel Llorens <daniel.llorens@bluewin.ch> writes:

> In 2.0.7
>
> scheme@(guile-user)> (import (ice-9 format))
> scheme@(guile-user)> (format #t "~!")
> ;;; <stdin>:2:0: warning: "~!": wrong number of `format' arguments: expected 1, got 0
> $1 = #t
> scheme@(guile-user)> (format #t "~!" 3)
> $2 = #t
> scheme@(guile-user)> (format #t "~!~a" 3 9)  
> 3$3 = #t
> scheme@(guile-user)> (format #t "~a~!" 3 9)  
> 3$4 = #t
> scheme@(guile-user)> (version)
> $5 = "2.0.7.28-581f4"
>
> So it doesn't take an argument, it only affects the warning.
Okay, I've confirmed this, and as far as I've been able to determine you
are correct. The problem only seems to be with the warning, not format
itself.

In format-string-argument-count in the module
module/language/tree-il/analyze.scm the ~! option gets picked up by the
else case of the loop, which adds one to both the max and minimum number
of parameters. Clearly this is a mistake.

I'm currently looking through the format docs to see if any others are
mishandled, and will post a patch later.


PS. I'm really glad this wasn't a bug in format, since that is one scary
function. :)

-- 
Ian Price -- shift-reset.com

"Programming is like pinball. The reward for doing it well is
the opportunity to do it again" - from "The Wizardy Compiled"

bug#13485: wrong warning for format ~!

[Ian Price]

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

Ian Price <ianprice90@googlemail.com> writes:

> I'm currently looking through the format docs to see if any others are
> mishandled, and will post a patch later.

So, having went through all of the docs for format, I think I've handled
all of the sequences correctly (except for the iteration ones, which
were already done, and I never double-checked). The two main issues were
sequences that don't take an argument updating the count, and some
sequences were not treated insensitively.

~^ confused me a little, but I think I have the correct behaviour for it

Ludovic,
are there test cases for this? I'm not sure where to look

-- 
Ian Price -- shift-reset.com

"Programming is like pinball. The reward for doing it well is
the opportunity to do it again" - from "The Wizardy Compiled"

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

From 986181b3b7bd7b6a0814ed45cb606238ab927db3 Mon Sep 17 00:00:00 2001
From: Ian Price <ianprice90@googlemail.com>
Date: Sat, 19 Jan 2013 17:05:27 +0000
Subject: [PATCH] Fix argument count for various format string escape
 sequences.

* module/language/tree-il/analyze.scm (format-string-argument-count):
  Handle ~t and ~k options case-insensitively.
  ~! ~| ~/ ~q and ~Q should not update the min-count or max-count.
  ~^ returns the min-count and 'any
---
 module/language/tree-il/analyze.scm |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/module/language/tree-il/analyze.scm b/module/language/tree-il/analyze.scm
index 88f81f3..29dd876 100644
--- a/module/language/tree-il/analyze.scm
+++ b/module/language/tree-il/analyze.scm
@@ -1259,7 +1259,7 @@ accurate information is missing from a given `tree-il' element."
         (case state
           ((tilde)
            (case (car chars)
-             ((#\~ #\% #\& #\t #\_ #\newline #\( #\))
+             ((#\~ #\% #\& #\t #\T #\_ #\newline #\( #\) #\! #\| #\/ #\q #\Q)
                         (loop (cdr chars) 'literal '()
                               conditions end-group
                               min-count max-count))
@@ -1330,10 +1330,12 @@ accurate information is missing from a given `tree-il' element."
                                      min-count)
                                   (+ (or (previous-number params) 1)
                                      max-count))))
-             ((#\? #\k)
+             ((#\? #\k #\K)
               ;; We don't have enough info to determine the exact number
               ;; of args, but we could determine a lower bound (TODO).
               (values 'any 'any))
+             ((#\^)
+              (values min-count 'any))
              ((#\h #\H)
                         (let ((argc (if (memq #\: params) 2 1)))
                           (loop (cdr chars) 'literal '()
-- 
1.7.7.6

bug#13485: wrong warning for format ~!

[Ian Price]

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

Ian Price <ianprice90@googlemail.com> writes:

>> I'm currently looking through the format docs to see if any others are
>> mishandled, and will post a patch later.
>
> So, having went through all of the docs for format, I think I've handled
> all of the sequences correctly (except for the iteration ones, which
> were already done, and I never double-checked).

Okay, spoke too soon. I forgot about the ' # + - parts of parameters.
I am less sure of these, but I have attached a patch.

-- 
Ian Price -- shift-reset.com

"Programming is like pinball. The reward for doing it well is
the opportunity to do it again" - from "The Wizardy Compiled"

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

From f479d801659708d04bf23d26bc81600d68282e18 Mon Sep 17 00:00:00 2001
From: Ian Price <ianprice90@googlemail.com>
Date: Sat, 19 Jan 2013 17:40:24 +0000
Subject: [PATCH] Fix escape sequence parameter handling in
 format-string-argument-count

* module/language/tree-il/analyze.scm (format-string-argument-count):
  + - # and ' should not increase the argument count.
---
 module/language/tree-il/analyze.scm |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/module/language/tree-il/analyze.scm b/module/language/tree-il/analyze.scm
index 29dd876..badce9f 100644
--- a/module/language/tree-il/analyze.scm
+++ b/module/language/tree-il/analyze.scm
@@ -1263,7 +1263,7 @@ accurate information is missing from a given `tree-il' element."
                         (loop (cdr chars) 'literal '()
                               conditions end-group
                               min-count max-count))
-             ((#\0 #\1 #\2 #\3 #\4 #\5 #\6 #\7 #\8 #\9 #\, #\: #\@)
+             ((#\0 #\1 #\2 #\3 #\4 #\5 #\6 #\7 #\8 #\9 #\, #\: #\@ #\+ #\- #\#)
                         (loop (cdr chars)
                               'tilde (cons (car chars) params)
                               conditions end-group
@@ -1342,6 +1342,11 @@ accurate information is missing from a given `tree-il' element."
                                 conditions end-group
                                 (+ argc min-count)
                                 (+ argc max-count))))
+             ((#\')
+              (if (null? (cdr chars))
+                  (throw &syntax-error 'unexpected-termination)
+                  (loop (cddr chars) 'tilde (cons (cadr chars) params)
+                        conditions end-group min-count max-count)))
              (else      (loop (cdr chars) 'literal '()
                               conditions end-group
                               (+ 1 min-count) (+ 1 max-count)))))
-- 
1.7.7.6

bug#13485: wrong warning for format ~!

[Ludovic Courtès]

Hi!

Ian Price <ianprice90@googlemail.com> skribis:

> So, having went through all of the docs for format, I think I've handled
> all of the sequences correctly (except for the iteration ones, which
> were already done, and I never double-checked). The two main issues were
> sequences that don't take an argument updating the count, and some
> sequences were not treated insensitively.

Excellent.

> Ludovic,
> are there test cases for this? I'm not sure where to look

Yes, in tree-il.test.  Can you add them?

Thanks!

Ludo’.

bug#13485: wrong warning for format ~!

[Ian Price]

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

ludo@gnu.org (Ludovic Courtès) writes:

>> Ludovic,
>> are there test cases for this? I'm not sure where to look
>
> Yes, in tree-il.test.  Can you add them?

Attached a patch that squashes the previous two and adds tests. I still
may want to add an additional patch to get some better errors for
sequence parameters.

-- 
Ian Price -- shift-reset.com

"Programming is like pinball. The reward for doing it well is
the opportunity to do it again" - from "The Wizardy Compiled"


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

From 90baf8cdfe8ce356ee4720a012e0deb5a2cb5818 Mon Sep 17 00:00:00 2001
From: Ian Price <ianprice90@googlemail.com>
Date: Sat, 19 Jan 2013 17:05:27 +0000
Subject: [PATCH] Fix argument count for various format string escape
 sequences.

* module/language/tree-il/analyze.scm (format-string-argument-count):
  Handle ~t and ~k options case-insensitively.
  ~! ~| ~/ ~q and ~Q should not update the min-count or max-count.
  ~^ returns the min-count and 'any
  + - # and ' should not increase the argument count.
* test-suite/tests/tree-il.test (*): Tests for new parameters.
---
 module/language/tree-il/analyze.scm |   13 ++++++++++---
 test-suite/tests/tree-il.test       |   29 +++++++++++++++++++++++++++--
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/module/language/tree-il/analyze.scm b/module/language/tree-il/analyze.scm
index 88f81f3..badce9f 100644
--- a/module/language/tree-il/analyze.scm
+++ b/module/language/tree-il/analyze.scm
@@ -1259,11 +1259,11 @@ accurate information is missing from a given `tree-il' element."
         (case state
           ((tilde)
            (case (car chars)
-             ((#\~ #\% #\& #\t #\_ #\newline #\( #\))
+             ((#\~ #\% #\& #\t #\T #\_ #\newline #\( #\) #\! #\| #\/ #\q #\Q)
                         (loop (cdr chars) 'literal '()
                               conditions end-group
                               min-count max-count))
-             ((#\0 #\1 #\2 #\3 #\4 #\5 #\6 #\7 #\8 #\9 #\, #\: #\@)
+             ((#\0 #\1 #\2 #\3 #\4 #\5 #\6 #\7 #\8 #\9 #\, #\: #\@ #\+ #\- #\#)
                         (loop (cdr chars)
                               'tilde (cons (car chars) params)
                               conditions end-group
@@ -1330,16 +1330,23 @@ accurate information is missing from a given `tree-il' element."
                                      min-count)
                                   (+ (or (previous-number params) 1)
                                      max-count))))
-             ((#\? #\k)
+             ((#\? #\k #\K)
               ;; We don't have enough info to determine the exact number
               ;; of args, but we could determine a lower bound (TODO).
               (values 'any 'any))
+             ((#\^)
+              (values min-count 'any))
              ((#\h #\H)
                         (let ((argc (if (memq #\: params) 2 1)))
                           (loop (cdr chars) 'literal '()
                                 conditions end-group
                                 (+ argc min-count)
                                 (+ argc max-count))))
+             ((#\')
+              (if (null? (cdr chars))
+                  (throw &syntax-error 'unexpected-termination)
+                  (loop (cddr chars) 'tilde (cons (cadr chars) params)
+                        conditions end-group min-count max-count)))
              (else      (loop (cdr chars) 'literal '()
                               conditions end-group
                               (+ 1 min-count) (+ 1 max-count)))))
diff --git a/test-suite/tests/tree-il.test b/test-suite/tests/tree-il.test
index 68dfc32..2217ffc 100644
--- a/test-suite/tests/tree-il.test
+++ b/test-suite/tests/tree-il.test
@@ -1415,11 +1415,11 @@
               (number? (string-contains (car w)
                                         "wrong number of arguments")))))
 
-     (pass-if "~%, ~~, ~&, ~t, ~_, and ~\\n"
+     (pass-if "~%, ~~, ~&, ~t, ~_, ~!, ~|, ~/, ~q and ~\\n"
        (null? (call-with-warnings
                (lambda ()
                  (compile '((@ (ice-9 format) format) some-port
-                            "~&~3_~~ ~\n~12they~%")
+                            "~&~3_~~ ~\n~12they~% ~!~|~/~q")
                           #:opts %opts-w-format
                           #:to 'assembly)))))
 
@@ -1687,6 +1687,31 @@
                           #:opts %opts-w-format
                           #:to 'assembly)))))
 
+     (pass-if "~^"
+       (null? (call-with-warnings
+               (lambda ()
+                 (compile '((@ (ice-9 format) format) #f "~a ~^ ~a" 0 1)
+                          #:opts %opts-w-format
+                          #:to 'assembly)))))
+
+     (pass-if "~^, too few args"
+       (let ((w (call-with-warnings
+                 (lambda ()
+                   (compile '((@ (ice-9 format) format) #f "~a ~^ ~a")
+                            #:opts %opts-w-format
+                            #:to 'assembly)))))
+         (and (= (length w) 1)
+              (number? (string-contains (car w)
+                                        "expected at least 1, got 0")))))
+
+     (pass-if "parameters: +,-,#, and '"
+       (null? (call-with-warnings
+               (lambda ()
+                 (compile '((@ (ice-9 format) format) some-port
+                            "~#~ ~,,-2f ~,,+2f ~'A~" 1234 1234)
+                          #:opts %opts-w-format
+                          #:to 'assembly)))))
+
      (pass-if "complex 1"
        (let ((w (call-with-warnings
                  (lambda ()
-- 
1.7.7.6

bug#13485: wrong warning for format ~!

[Ludovic Courtès]

Ian Price <ianprice90@googlemail.com> skribis:

> Attached a patch that squashes the previous two and adds tests.

Looks good to me, feel free to push.

> I still may want to add an additional patch to get some better errors
> for sequence parameters.

That can be done in a separate patch.

Thanks!

Ludo’.

bug#13485: wrong warning for format ~!

[Andy Wingo]

Fixed in 90baf8cdfe8ce356ee4720a012e0deb5a2cb5818 by ijp.  Thanks to
all!
-- 
http://wingolog.org/