public inbox for cgen@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] fix handling of --help
@ 2021-09-24 14:25 Andrew Burgess
  2021-09-24 14:38 ` Frank Ch. Eigler
  0 siblings, 1 reply; 2+ messages in thread
From: Andrew Burgess @ 2021-09-24 14:25 UTC (permalink / raw)
  To: cgen

This issue was reported here:

  https://sourceware.org/pipermail/cgen/2021q2/002768.html

Possible fix is below.  I would describe my scheme skills as
"enthusiastic amateur", so I'm always happy to improve and learn if
people have constructive feedback.

Thanks,
Andrew

---

The --help option is currently broken:

  $ guile -l ./guile.scm -s ./cgen-sim.scm -s . --help

  Backtrace:
  In ././read.scm:
  1145:  0* [string-append "
  " "                    " #f]
  In unknown file:
     ?:  1* [#<procedure #f (text)> #f]
  In ././read.scm:
  1144:  2* [map #<procedure #f (text)> (#f #<procedure #f (arg)>)]
  1143:  3* [apply #<primitive-procedure string-append> ...
  1136:  4* [string-append "-A file         " "  - " "generate arch.h in <file>" ...
  1136:  5* [display ...
  In unknown file:
     ?:  6* [#<procedure #f (arg)> ("-A" "file" "generate arch.h in <file>" ...)]
  In ././read.scm:
  1135:  7* [for-each #<procedure #f (arg)> (# # # # ...)]
  1128:  8  (let* ((cep (current-error-port))) (case errtype ((help) #f) ...) ...)
  1334:  9  [cgen-usage help "" ...]
  1314: 10  (cond (# #) (# #) (# #) ...)
  1309: 11* (case opt ((#f) (set! moreopts? #f)) ...)
  1306: 12  (let* ((new-argv #) (opt #) (arg #)) (case opt (# #) ...) ...)
  1377: 13  [loop ("--help")]
  1377: 14  (if moreopts? (loop (cdr new-argv)))
  1306: 15  (let* ((new-argv #) (opt #) (arg #)) (case opt (# #) ...) ...)
  1305: 16* [loop ("-s" "." "--help")]
  1289: 17  (let (# # # # ...) (# #) (cgen-call-with-debugging debugging #))
  1257: 18* (let (# # # # ...) (# args) (let # # #))

The cause of this failure is a disagreement about how command line
arguments are held within cgen.

The arguments are split into two groups, in read.scm we have
common-arguments, these are arguments for all cgen applications.  this
is a list of arguments, each argument is itself a list.

The format of each argument list is:

  1.  The argument flag, e.g. '-a',
  2.  The name of any parameter that the argument takes, or #f if the
      argument doesn't take a parameter.
  3.  A short description of the argument, and
  4+. Optionally, one or more additional lines of description for the
      argument.

The application specific arguments though are found in the application
files, e.g. in cgen-sim.scm we have sim-arguments.

These application specific arguments are also a list of arguments,
with each argument being a list.  However, the each argument list has
a different layout to the common arguments:

  1.  The argument flag, e.g. '-a',
  2.  The name of any parameter that the argument takes, or #f if the
      argument doesn't take a parameter.
  3.  A short description of the argument, and
  4.  A first pass callback,
  5.  A second pass callback.

The two callback arguments are used to implement the functionality of
each argument.

And so, the problem is, I hope, clear.  When we try to display the
usage, the cgen-usage code expects arguments 4 and 5, if present, to
be strings, which, for the application specific arguments they are
not, and so we get an error.

My proposed fix for this is to change the format of the common
arguments, so we now have:

  1.  The argument flag, e.g. '-a',
  2.  The name of any parameter that the argument takes, or #f if the
      argument doesn't take a parameter.
  3.  EITHER (a) a single string, a short description, or
      (b) a list, the first item of which is a short description, and
          the remaining items are the extra lines of description.

I then update the cgen-usage code to take this new argument format
into account, and we're almost done.

Now that the complete help text is being displayed, I still get a
backtrace printed.

The problem is that, after displaying the --help text cgen call quit,
like this (quit 0).

However, this quit is still caught inside cgen-debugging-stack-start,
which then displays a backtrace.

So, in this commit I also catch, and special case, the situation where
cgen-debugging-stack-start catches '(quit 0).  I assume that if we get
this exception then this indicates that cgen wants to quit, and that
this is not an error condition, so, we simply re-quit, and avoid
printing any backtrace.

Now I see this behaviour:

  $ guile -l ./guile.scm -s ./cgen-sim.scm -s . --help
  cgen -s ./cgen-sim.scm -s . --help
  Usage: cgen arguments ...
  -a arch-file      - specify path of .cpu file to load
  -b                - use debugging evaluator, for backtraces
  -d                - start interactive debugging session
  -f flags          - specify a set of flags to control code generation
  -h                - print usage information
  --help            - print usage information
  -i isa-list       - specify isa-list entries to keep
  -m mach-list      - specify mach-list entries to keep
  -s srcdir         - set srcdir
  -t trace-options  - specify list of things to trace
                      Options:
                      commands - trace cgen commands (e.g. define-insn)
                      pmacros  - trace pmacro expansion
                      all      - trace everything
  -v                - increment verbosity level
  -w diagnostic-options  - specify list of things to issue diagnostics about
                      Options:
                      iformat - verify instruction formats are valid
                      all     - turn on all diagnostics
  --version         - print version info
  -A file           - generate arch.h in <file>
  -B file           - generate arch.c in <file>
  -C file           - generate cpu-<cpu>.h in <file>
  -U file           - generate cpu-<cpu>.c in <file>
  -N file           - generate cpu-all.h in <file>
  -F file           - generate memops.h in <file>
  -G file           - generate defs.h in <file>
  -P file           - generate semops.h in <file>
  -T file           - generate decode.h in <file>
  -D file           - generate decode.c in <file>
  -E file           - generate extract.c in <file>
  -R file           - generate read.c in <file>
  -W file           - generate write.c in <file>
  -S file           - generate semantics.c in <file>
  -X file           - generate sem-switch.c in <file>
  -O file           - generate ops.c in <file>
  -M file           - generate model.c in <file>
  -L file           - generate mainloop.in in <file>
  ...

Much better.

ChangeLog:

	* guile.scm (cgen-debugging-stack-start): In the exception
	handler, if the exception we get is '(quit 0) then assume this is
	not really an exception, and just exit silently, without
	displaying a backtrace.
	* read.scm (cgen-usage): Update processing of arguments list to
	handle new configurations.
	(common-arguments): Only the first 3 elements of each help item
	are for help text, the 4th and 5th elements are for function
	callbacks, used by the application specific arguments.
---
 ChangeLog | 12 ++++++++++++
 guile.scm | 14 ++++++++++++--
 read.scm  | 45 ++++++++++++++++++++++++++-------------------
 3 files changed, 50 insertions(+), 21 deletions(-)

diff --git a/guile.scm b/guile.scm
index d2b8d8d..7290de7 100644
--- a/guile.scm
+++ b/guile.scm
@@ -126,8 +126,18 @@
   ;; Naming this procedure, rather than using an anonymous lambda,
   ;; allows us to pass less fragile cut info to save-stack.
   (define (handler . args)
-		;;(display args (current-error-port))
-		;;(newline (current-error-port))
+		;; If the "exception" we got is actually just (quit
+		;; 0), i.e. a normal exit, then don't display a
+		;; backtrace, instead, just exit normally.
+		;;
+		;; For any other quit we assume that this is bad, and
+		;; so do display the backtrace.
+		(if (and (list? args)
+                         (eq? (car args) 'quit)
+                         (eq? (cadr args) 0))
+                    (quit 0))
+		;; (display args (current-error-port))
+		    (newline (current-error-port))
 		;; display-error takes 6 arguments.
 		;; If `quit' is called from elsewhere, it may not have 6
 		;; arguments.  Not sure how best to handle this.
diff --git a/read.scm b/read.scm
index ee3f488..2b48cee 100644
--- a/read.scm
+++ b/read.scm
@@ -1133,20 +1133,27 @@ Define a preprocessor-style macro.
       (else (display "Unknown error!\n" cep)))
     (display "Usage: cgen arguments ...\n" cep)
     (for-each (lambda (arg)
-		(display (string-append
-			  (let ((arg-str (string-append (car arg) " "
-							(or (cadr arg) ""))))
-			    (if (< (string-length arg-str) 16)
-				(string-take 16 arg-str)
-				arg-str))
-			  "  - " (caddr arg)
-			  (apply string-append
-				 (map (lambda (text)
-					(string-append "\n"
-						       (string-take 20 "")
-						       text))
-				      (cdddr arg)))
-			  "\n")
+		(display (apply
+                          string-append
+                          (append
+                           (list
+                            (let ((arg-str (string-append (car arg) " "
+                                                          (or (cadr arg) ""))))
+                              (if (< (string-length arg-str) 16)
+                                  (string-take 16 arg-str)
+                                  arg-str))
+                            "  - ")
+                          (let ((desc (caddr arg)))
+                            (if (list? desc)
+                                (append (list (car desc))
+                                        (map (lambda (text)
+                                               (string-append
+                                                "\n"
+                                                (string-take 20 "")
+                                                text))
+                                             (cdr desc)))
+                                (list desc)))
+			  (list "\n")))
 			 cep))
 	      arguments)
     (display "...\n" cep)
@@ -1202,7 +1209,7 @@ Define a preprocessor-style macro.
 ;; arguments specified up til now, then continue with next batch of args".
 
 (define common-arguments
-  '(("-a" "arch-file" "specify path of .cpu file to load")
+  `(("-a" "arch-file" "specify path of .cpu file to load")
     ("-b" #f          "use debugging evaluator, for backtraces")
     ("-d" #f          "start interactive debugging session")
     ("-f" "flags"     "specify a set of flags to control code generation")
@@ -1211,16 +1218,16 @@ Define a preprocessor-style macro.
     ("-i" "isa-list"  "specify isa-list entries to keep")
     ("-m" "mach-list" "specify mach-list entries to keep")
     ("-s" "srcdir"    "set srcdir")
-    ("-t" "trace-options" "specify list of things to trace"
+    ("-t" "trace-options" ,(list "specify list of things to trace"
                        "Options:"
                        "commands - trace cgen commands (e.g. define-insn)"
                        "pmacros  - trace pmacro expansion"
-		       "all      - trace everything")
+		       "all      - trace everything"))
     ("-v" #f          "increment verbosity level")
-    ("-w" "diagnostic-options" "specify list of things to issue diagnostics about"
+    ("-w" "diagnostic-options" ,(list "specify list of things to issue diagnostics about"
                        "Options:"
                        "iformat - verify instruction formats are valid"
-		       "all     - turn on all diagnostics")
+		       "all     - turn on all diagnostics"))
 
     ("--version" #f   "print version info")
     )
-- 
2.25.4


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] fix handling of --help
  2021-09-24 14:25 [PATCH] fix handling of --help Andrew Burgess
@ 2021-09-24 14:38 ` Frank Ch. Eigler
  0 siblings, 0 replies; 2+ messages in thread
From: Frank Ch. Eigler @ 2021-09-24 14:38 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: cgen

Hi -

> Possible fix is below.  I would describe my scheme skills as
> "enthusiastic amateur", so I'm always happy to improve and learn if
> people have constructive feedback.

lgtm, thanks!  Feel free to push if able.

- FChE


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-09-24 14:38 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-24 14:25 [PATCH] fix handling of --help Andrew Burgess
2021-09-24 14:38 ` Frank Ch. Eigler

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).