Cleaning up LLVM IR mode for Emacs

[Wilfred]

Summary

I've made a range of improvements to the Emacs mode for LLVM IR.

Most importantly, it changes llvm-mode to inherit from prog-mode. This means llvm-mode will be treated as a normal programming mode in Emacs, so many Emacs features will just work. prog-mode is new to Emacs 24, so I've added an alias to ensure compatibility with Emacs 23 too.

I've changed the mode definition to use define-derived-mode. This saves us needing to set up local variables ourselves, and saves us needing to define llvm-mode-map, llvm-mode-abbrev-table, llvm-mode-map.

I've removed the keybindings to tab-to-tab-stop, center-line and center-paragraph. This shouldn't be llvm-mode's responsibility, and the code didn't actually work anyway (since (not llvm-mode-map) always evaluated to t, the keybindings were never executed).

I've simplified the syntax-table definition, it's equivalent (e.g. " is treated as string delimiter by default in Emacs). I've added . as a symbol constituent, so functions like llvm.memset.p0i8.i32 are recognised as a single symbol. I've also changed % to be a symbol constituent, so users can move between words or symbols at their choice, rather than conflating the two.

I've fixed regexp for types, which incorrect used symbol instead of symbols as an argument to regexp-opt. This was causing incorrect highlighting on lines like call void @foovoid.

I've removed string and comment highlighting from llvm-font-lock-keywords. This is already handled by the syntax-table.

Finally, I've removed the reference to jasmin. That project is long abandoned and the link 404s. For reference, I've found an old copy of the project here: https://github.com/stevej/emacs/blob/master/vendor/jasmin/jasmin.el

[• rafael]

I think this is fine, but don't know emacs internals, so I added Ramkumar as a reviewer (he did some recent changes).

[artagnon]

LGTM, with some minor doubts.

utils/emacs/llvm-mode.el

11–17

I _think_ this is okay.

72

A good change, for sure.

[Wilfred]

utils/emacs/llvm-mode.el

11–17

Let me try to put your mind at ease :)

We started with:

(defvar llvm-mode-syntax-table nil
  "Syntax table used while in LLVM mode.")
(if (not llvm-mode-syntax-table)
    (progn
      (setq llvm-mode-syntax-table (make-syntax-table))
      (mapc (function (lambda (n)
                        (modify-syntax-entry (aref n 0)
                                             (aref n 1)
                                             llvm-mode-syntax-table)))
            '(
              ;; whitespace (` ')
              [?\^m " "]
              [?\f  " "]
              [?\n  " "]
              [?\t  " "]
              [?\   " "]
              ;; word constituents (`w')
              ;;[?<  "w"]
              ;;[?>  "w"]
              [?%  "w"]
              ;;[?_  "w  "]
              ;; comments
              [?\;  "< "]
              [?\n  "> "]
              ;;[?\r  "> "]
              ;;[?\^m "> "]
              ;; symbol constituents (`_')
              ;; punctuation (`.')
              ;; open paren (`(')
              ;; close paren (`)')
              ;; string quote ('"')
              [?\" "\""]))))

This style of defining a variable and setting it afterwards is intended to make it easier to repeatedly evaluate the expression when developing the code. However, if you use edebug-eval-defun, your expression is re-evaluated anyway, allowing you to use defvar directly:

(defvar llvm-mode-syntax-table
  (progn
    (setq llvm-mode-syntax-table (make-syntax-table))
    (mapc (function (lambda (n)
                      (modify-syntax-entry (aref n 0)
                                           (aref n 1)
                                           llvm-mode-syntax-table)))
          '(
            ;; whitespace (` ')
            [?\^m " "]
            [?\f  " "]
            [?\n  " "]
            [?\t  " "]
            [?\   " "]
            ;; word constituents (`w')
            ;;[?<  "w"]
            ;;[?>  "w"]
            [?%  "w"]
            ;;[?_  "w  "]
            ;; comments
            [?\;  "< "]
            [?\n  "> "]
            ;;[?\r  "> "]
            ;;[?\^m "> "]
            ;; symbol constituents (`_')
            ;; punctuation (`.')
            ;; open paren (`(')
            ;; close paren (`)')
            ;; string quote ('"')
            [?\" "\""]))
    llvm-mode-syntax-table)
  "Syntax table used while in LLVM mode.")

Remove comments and commented-out expressions:

(defvar llvm-mode-syntax-table
  (progn
    (setq llvm-mode-syntax-table (make-syntax-table))
    (mapc (function (lambda (n)
                      (modify-syntax-entry (aref n 0)
                                           (aref n 1)
                                           llvm-mode-syntax-table)))
          '([?\^m " "]
            [?\f  " "]
            [?\n  " "]
            [?\t  " "]
            [?\   " "]
            [?%  "w"]
            [?\;  "< "]
            [?\n  "> "]
            [?\" "\""]))
    llvm-mode-syntax-table)
  "Syntax table used while in LLVM mode.")

Just call the function directly instead of using map:

(defvar llvm-mode-syntax-table
  (let ((table (make-syntax-table)))
    (modify-syntax-entry ?\^m " " table)
    (modify-syntax-entry ?\f  " " table)
    (modify-syntax-entry ?\n  " " table)
    (modify-syntax-entry ?\t  " " table)
    (modify-syntax-entry ?\   " " table)
    (modify-syntax-entry ?%  "w" table)
    (modify-syntax-entry ?\;  "< " table)
    (modify-syntax-entry ?\n  "> " table)
    (modify-syntax-entry ?\" "\"" table)
    table)
  "Syntax table used while in LLVM mode.")

Whitespace characters are treated as whitespace syntax by
default. Similarly, the default syntax table treats " as a string
delimiter by default. We can thus reduce our syntax table:

(defvar llvm-mode-syntax-table
  (let ((table (make-syntax-table)))
    (modify-syntax-entry ?%  "w" table)
    (modify-syntax-entry ?\;  "< " table)
    (modify-syntax-entry ?\n  "> " table)
    table)
  "Syntax table used while in LLVM mode.")

It's good practice to only treat [a-zA-Z0-9] as word constituents in Emacs. Given a variable foo_bar_baz, it's better to treat _ as a symbol constituent so users can choose to move by word (foo to bar) or by symbol as they wish. We change % to be a symbol constituent:

(defvar llvm-mode-syntax-table
  (let ((table (make-syntax-table)))
    (modify-syntax-entry ?%  "_" table)
    (modify-syntax-entry ?\;  "< " table)
    (modify-syntax-entry ?\n  "> " table)
    table)
  "Syntax table used while in LLVM mode.")

Finally, since . can occur in function names (e.g. llvm.memset.p0i8.i32), treat that as a symbol constituent, so we highlight such functions correctly.

(defvar llvm-mode-syntax-table
  (let ((table (make-syntax-table)))
    (modify-syntax-entry ?%  "_" table)
    (modify-syntax-entry ?. "_" table table)
    (modify-syntax-entry ?\;  "< " table)
    (modify-syntax-entry ?\n  "> " table)
    table)
  "Syntax table used while in LLVM mode.")

[Wilfred]

I don't have commit access -- would one of you fine folks be willing to apply this patch?

[• rafael]

r247281.

utils/emacs/llvm-mode.el

 
 ;;; Code:
 
-(defvar llvm-mode-syntax-table nil
+(defvar llvm-mode-syntax-table
+  (let ((table (make-syntax-table)))
+    (modify-syntax-entry ?% "w" table)
+    (modify-syntax-entry ?. "w" table)
+    (modify-syntax-entry ?\; "< " table)
+    (modify-syntax-entry ?\n "> " table)
+    table)
   "Syntax table used while in LLVM mode.")
+
 (defvar llvm-font-lock-keywords
   (list
-   ;; Comments
-   '(";.*" . font-lock-comment-face)
    ;; Variables
    '("%[-a-zA-Z$\._][-a-zA-Z$\._0-9]*" . font-lock-variable-name-face)
    ;; Labels
    '("[-a-zA-Z$\._0-9]+:" . font-lock-variable-name-face)
-   ;; Strings
-   '("\"[^\"]+\"" . font-lock-string-face)
    ;; Unnamed variable slots
    '("%[-]?[0-9]+" . font-lock-variable-name-face)
    ;; Types
-   `(,(regexp-opt '("void" "i1" "i8" "i16" "i32" "i64" "i128" "float" "double" "type" "label" "opaque") 'symbol) . font-lock-type-face)
+   `(,(regexp-opt '("void" "i1" "i8" "i16" "i32" "i64" "i128" "float" "double" "type" "label" "opaque") 'symbols) . font-lock-type-face)
    ;; Integer literals
    '("\\b[-]?[0-9]+\\b" . font-lock-preprocessor-face)
    ;; Floating point constants

    ;; Metadata types
    `(,(regexp-opt '("distinct") 'symbols) . font-lock-keyword-face)
    ;; Use-list order directives
-   `(,(regexp-opt '("uselistorder" "uselistorder_bb") 'symbols) . font-lock-keyword-face)
-
-   )
-  "Syntax highlighting for LLVM."
-  )
-
-;; ---------------------- Syntax table ---------------------------
-;; Shamelessly ripped from jasmin.el
-;; URL: http://www.neilvandyke.org/jasmin-emacs/jasmin.el.html
-
-(if (not llvm-mode-syntax-table)
-    (progn
-      (setq llvm-mode-syntax-table (make-syntax-table))
-      (mapc (function (lambda (n)
-                        (modify-syntax-entry (aref n 0)
-                                             (aref n 1)
-                                             llvm-mode-syntax-table)))
-            '(
-              ;; whitespace (` ')
-              [?\^m " "]
-              [?\f  " "]
-              [?\n  " "]
-              [?\t  " "]
-              [?\   " "]
-              ;; word constituents (`w')
-              ;;[?<  "w"]
-              ;;[?>  "w"]
-              [?%  "w"]
-              ;;[?_  "w  "]
-              ;; comments
-              [?\;  "< "]
-              [?\n  "> "]
-              ;;[?\r  "> "]
-              ;;[?\^m "> "]
-              ;; symbol constituents (`_')
-              ;; punctuation (`.')
-              ;; open paren (`(')
-              ;; close paren (`)')
-              ;; string quote ('"')
-              [?\" "\""]))))
-
-;; --------------------- Abbrev table -----------------------------
+   `(,(regexp-opt '("uselistorder" "uselistorder_bb") 'symbols) . font-lock-keyword-face))
+  "Syntax highlighting for LLVM.")
 
-(defvar llvm-mode-abbrev-table nil
-  "Abbrev table used while in LLVM mode.")
-(define-abbrev-table 'llvm-mode-abbrev-table ())
-
-(defvar llvm-mode-hook nil)
-(defvar llvm-mode-map nil)   ; Create a mode-specific keymap.
-
-(if (not llvm-mode-map)
-    ()  ; Do not change the keymap if it is already set up.
-  (setq llvm-mode-map (make-sparse-keymap))
-  (define-key llvm-mode-map "\t" 'tab-to-tab-stop)
-  (define-key llvm-mode-map "\es" 'center-line)
-  (define-key llvm-mode-map "\eS" 'center-paragraph))
+;; Emacs 23 compatibility.
+(defalias 'llvm-mode-prog-mode
+  (if (fboundp 'prog-mode)
+      'prog-mode
+    'fundamental-mode))
 
 ;;;###autoload
-(defun llvm-mode ()
+(define-derived-mode llvm-mode llvm-mode-prog-mode "LLVM"
   "Major mode for editing LLVM source files.
 \\{llvm-mode-map}
   Runs `llvm-mode-hook' on startup."
-  (interactive)
-  (kill-all-local-variables)
-  (use-local-map llvm-mode-map)         ; Provides the local keymap.
-  (setq major-mode 'llvm-mode)
-
-  (make-local-variable 'font-lock-defaults)
-  (setq major-mode 'llvm-mode           ; This is how describe-mode
-                                        ;   finds the doc string to print.
-  mode-name "LLVM"                      ; This name goes into the modeline.
-  font-lock-defaults `(llvm-font-lock-keywords))
-
-  (setq local-abbrev-table llvm-mode-abbrev-table)
-  (set-syntax-table llvm-mode-syntax-table)
-  (setq comment-start ";")
-  (run-hooks 'llvm-mode-hook))          ; Finally, this permits the user to
-                                        ;   customize the mode with a hook.
+  (setq font-lock-defaults `(llvm-font-lock-keywords))
+  (setq comment-start ";"))
 
 ;; Associate .ll files with llvm-mode
 ;;;###autoload