Optimizations for flymake

Optimizations for flymake

[Kim F. Storm]

Hi Pavel,

The byte-compiler explicitly knows how to handle (i.e. eliminate)
code conditioned by (if (featurep 'xemacs) ...) and it is also
faster to use defsubst for trivial functions.

Here is a patch to flymake.el which uses these optimizations, and also
passes the byte-compiler without warnings (except for makehash which is
obsolete in 21.4):


*** flymake.el	01 Nov 2004 23:10:05 +0100	1.4
--- flymake.el	01 Nov 2004 23:32:52 +0100	
***************
*** 36,83 ****
  (provide 'flymake)
  
  ;;;; [[ Overlay compatibility
  (autoload 'make-overlay            "overlay" "Overlay compatibility kit." t)
  (autoload 'overlayp                "overlay" "Overlay compatibility kit." t)
  (autoload 'overlays-in             "overlay" "Overlay compatibility kit." t)
  (autoload 'delete-overlay          "overlay" "Overlay compatibility kit." t)
  (autoload 'overlay-put             "overlay" "Overlay compatibility kit." t)
  (autoload 'overlay-get             "overlay" "Overlay compatibility kit." t)
  ;;;; ]]
  
  ;;;; [[ cross-emacs compatibility routines
! (defvar flymake-emacs
!     (cond
! 	((string-match "XEmacs" emacs-version)  'xemacs)
! 	(t                                      'emacs)
! 	)
!     "Currently used emacs flavor"
! )
! 
! (defun flymake-makehash(&optional test)
!     (cond
! 	((equal flymake-emacs 'xemacs)  (if test (make-hash-table :test test) (make-hash-table)))
! 	(t                              (makehash test))	
!     )
! )
  
- (defun flymake-time-to-float(&optional tm)
- 	"Convert `current-time` to a float number of seconds."
- 	(multiple-value-bind (s0 s1 s2) (or tm (current-time))
- 	  (+ (* (float (ash 1 16)) s0) (float s1) (* 0.0000001 s2)))
- )
  (defun flymake-float-time()
!     (cond
! 	((equal flymake-emacs 'xemacs)  (flymake-time-to-float (current-time)))
! 	(t                              (float-time))
!     )
! )
! 
! (defun flymake-replace-regexp-in-string(regexp rep str)
!     (cond
! 	((equal flymake-emacs 'xemacs)  (replace-in-string str regexp rep))
! 	(t                              (replace-regexp-in-string regexp rep str))
! 	)
! )
  
  (defun flymake-split-string-remove-empty-edges(str pattern)
      "split, then remove first and/or last in case it's empty"
--- 36,68 ----
  (provide 'flymake)
  
  ;;;; [[ Overlay compatibility
+ (if (featurep 'xemacs) (progn
  (autoload 'make-overlay            "overlay" "Overlay compatibility kit." t)
  (autoload 'overlayp                "overlay" "Overlay compatibility kit." t)
  (autoload 'overlays-in             "overlay" "Overlay compatibility kit." t)
  (autoload 'delete-overlay          "overlay" "Overlay compatibility kit." t)
  (autoload 'overlay-put             "overlay" "Overlay compatibility kit." t)
  (autoload 'overlay-get             "overlay" "Overlay compatibility kit." t)
+ )) ;; xemacs
  ;;;; ]]
  
  ;;;; [[ cross-emacs compatibility routines
! (defsubst flymake-makehash(&optional test)
!   (if (featurep 'xemacs)
!       (if test (make-hash-table :test test) (make-hash-table))
!     (makehash test)))
  
  (defun flymake-float-time()
!     (if (featurep 'xemacs)
! 	(let ((tm (current-time)))
! 	  (multiple-value-bind (s0 s1 s2) (current-time)
! 			       (+ (* (float (ash 1 16)) s0) (float s1) (* 0.0000001 s2))))
!       (float-time)))
! 
! (defsubst flymake-replace-regexp-in-string(regexp rep str)
!     (if (featurep 'xemacs)
! 	(replace-in-string str regexp rep)
!       (replace-regexp-in-string regexp rep str)))
  
  (defun flymake-split-string-remove-empty-edges(str pattern)
      "split, then remove first and/or last in case it's empty"
***************
*** 91,109 ****
  		splitted
      )
  )
! (defun flymake-split-string(str pattern)
!     (cond
! 	((equal flymake-emacs 'xemacs)  (flymake-split-string-remove-empty-edges str pattern))
! 	(t                              (flymake-split-string-remove-empty-edges str pattern))
!     )
! )
! 
! (defun flymake-get-temp-dir()
!     (cond
! 	((equal flymake-emacs 'xemacs)  (temp-directory))
! 	(t                              temporary-file-directory)
!     )
! )
  
  (defun flymake-line-beginning-position()
      (save-excursion
--- 76,90 ----
  		splitted
      )
  )
! (defsubst flymake-split-string(str pattern)
!     (if (featurep 'xemacs)
! 	(flymake-split-string-remove-empty-edges str pattern)
!       (flymake-split-string-remove-empty-edges str pattern)))
! 
! (defsubst flymake-get-temp-dir()
!     (if (featurep 'xemacs)
! 	(temp-directory)
!       temporary-file-directory))
  
  (defun flymake-line-beginning-position()
      (save-excursion
***************
*** 120,138 ****
  )
  
  (defun flymake-popup-menu(pos menu-data)
!     (cond
!        ((equal flymake-emacs 'xemacs)
! 	    (let* ((x-pos       (nth 0 (nth 0 pos)))
! 		   (y-pos       (nth 1 (nth 0 pos)))
! 		   (fake-event-props  '(button 1 x 1 y 1)))
! 		(setq fake-event-props (plist-put fake-event-props 'x x-pos))
! 		(setq fake-event-props (plist-put fake-event-props 'y y-pos))
! 		(popup-menu (flymake-make-xemacs-menu menu-data) (make-event 'button-press fake-event-props))
! 	    )
!        )
!        (t                              (x-popup-menu pos (flymake-make-emacs-menu menu-data)))
!     )
! )
  
  (defun flymake-make-emacs-menu(menu-data)
      (let* ((menu-title     (nth 0 menu-data))
--- 101,115 ----
  )
  
  (defun flymake-popup-menu(pos menu-data)
!     (if (featurep 'xemacs)
! 	(let* ((x-pos       (nth 0 (nth 0 pos)))
! 	       (y-pos       (nth 1 (nth 0 pos)))
! 	       (fake-event-props  '(button 1 x 1 y 1)))
! 	  (setq fake-event-props (plist-put fake-event-props 'x x-pos))
! 	  (setq fake-event-props (plist-put fake-event-props 'y y-pos))
! 	  (popup-menu (flymake-make-xemacs-menu menu-data) (make-event 'button-press fake-event-props))
! 	  )
!       (x-popup-menu pos (flymake-make-emacs-menu menu-data))))
  
  (defun flymake-make-emacs-menu(menu-data)
      (let* ((menu-title     (nth 0 menu-data))
***************
*** 149,154 ****
--- 126,133 ----
  (defun flymake-nop()
  )
  
+ (if (featurep 'xemacs) (progn
+ 
  (defun flymake-make-xemacs-menu(menu-data)
      (let* ((menu-title     (nth 0 menu-data))
  	   (menu-items     (nth 1 menu-data))
***************
*** 175,194 ****
      )
  )
  
  (defun flymake-current-row()
      "return current row in current frame"
!     (cond
!        ((equal flymake-emacs 'xemacs)  (count-lines (window-start) (point)))
!        (t                              (+ (car (cdr (window-edges))) (count-lines (window-start) (point))))
!     )
! )
! 
! (defun flymake-selected-frame()
!     (cond
!        ((equal flymake-emacs 'xemacs)  (selected-window))
!        (t                              (selected-frame))
!     )
! )
  
  ;;;; ]]
  
--- 154,172 ----
      )
  )
  
+ )) ;; xemacs
+ 
+ 
  (defun flymake-current-row()
      "return current row in current frame"
!     (if (featurep 'xemacs)
! 	(count-lines (window-start) (point))
!       (+ (car (cdr (window-edges))) (count-lines (window-start) (point)))))
! 
! (defsubst flymake-selected-frame()
!     (if (featurep 'xemacs)
! 	(selected-window)
!       (selected-frame)))
  
  ;;;; ]]
  
***************
*** 547,552 ****
--- 525,534 ----
      :type 'integer
  )
  
+ (defvar flymake-included-file-name nil ; this is used to pass a parameter to a sort predicate below
+     ""
+ )
+ 
  (defun flymake-find-possible-master-files(file-name master-file-dirs masks)
      "find (by name and location) all posible master files, which are .cpp and .c for and .h.
  Files are searched for starting from the .h directory and max max-level parent dirs.
***************
*** 593,602 ****
      )
  )
  
- (defvar flymake-included-file-name nil ; this is used to pass a parameter to a sort predicate below
-     ""
- )
- 
  (defun flymake-master-file-compare(file-one file-two)
      "used in sort to move most possible file names to the beginning of the list (File.h -> File.cpp moved to top"
      (and (equal (file-name-sans-extension flymake-included-file-name)
--- 575,580 ----
***************
*** 1270,1276 ****
  	2 4 nil 5)
        )
  	 ;; compilation-error-regexp-alist)
!      (flymake-reformat-err-line-patterns-from-compile-el compilation-error-regexp-alist-alist)) 
      "patterns for matching error/warning lines, (regexp file-idx line-idx err-text-idx). Use flymake-reformat-err-line-patterns-from-compile-el to add patterns from compile.el"
  )
  ;(defcustom flymake-err-line-patterns
--- 1248,1254 ----
  	2 4 nil 5)
        )
  	 ;; compilation-error-regexp-alist)
!      (flymake-reformat-err-line-patterns-from-compile-el compilation-error-regexp-alist-alist))
      "patterns for matching error/warning lines, (regexp file-idx line-idx err-text-idx). Use flymake-reformat-err-line-patterns-from-compile-el to add patterns from compile.el"
  )
  ;(defcustom flymake-err-line-patterns
***************
*** 1959,1964 ****
--- 1937,1954 ----
      (setq minor-mode-alist (cons '(flymake-mode flymake-mode-line) minor-mode-alist))
  )
  
+ (defcustom flymake-start-syntax-check-on-newline t
+     "start syntax check if newline char was added/removed from the buffer"
+     :group 'flymake
+     :type 'boolean
+ )
+ 
+ (defcustom flymake-start-syntax-check-on-find-file t
+     "statr syntax check on find file"
+     :group 'flymake
+     :type 'boolean
+ )
+ 
  ;;;###autoload
  (defun flymake-mode-on()
      "turn flymake mode on"
***************
*** 2004,2015 ****
      )
  )
  
- (defcustom flymake-start-syntax-check-on-newline t
-     "start syntax check if newline char was added/removed from the buffer"
-     :group 'flymake
-     :type 'boolean
- )
- 
  (defun flymake-after-change-function(start stop len)
      "Start syntax check for current buffer if it isn't already running"
      ;+(flymake-log 0 "setting change time to %s" (flymake-float-time))
--- 1994,1999 ----
***************
*** 2038,2049 ****
      )
  )
  
- (defcustom flymake-start-syntax-check-on-find-file t
-     "statr syntax check on find file"
-     :group 'flymake
-     :type 'boolean
- )
- 
  (defun flymake-find-file-hook()
      ;+(when flymake-start-syntax-check-on-find-file
      ;+    (flymake-log 3 "starting syntax check on file open")
--- 2022,2027 ----



-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Optimizations for flymake

[Stefan]

> ! (defsubst flymake-makehash(&optional test)
> !   (if (featurep 'xemacs)
> !       (if test (make-hash-table :test test) (make-hash-table))
> !     (makehash test)))

Why not (if (fboundp 'make-hash-table) ...) ?
  
>   (defun flymake-float-time()
> !     (if (featurep 'xemacs)
> ! 	(let ((tm (current-time)))
> ! 	  (multiple-value-bind (s0 s1 s2) (current-time)
> ! 			       (+ (* (float (ash 1 16)) s0) (float s1) (* 0.0000001 s2))))
> !       (float-time)))

Why not (if (fboundp 'float-time) ...) ?

> ! (defsubst flymake-replace-regexp-in-string(regexp rep str)
> !     (if (featurep 'xemacs)
> ! 	(replace-in-string str regexp rep)
> !       (replace-regexp-in-string regexp rep str)))

Why not (if (fboundp 'replace-regexp-in-string) ...) ?
I'd actually make it even more efficient than `defsubst':

> ! (defsubst flymake-split-string(str pattern)
> !     (if (featurep 'xemacs)
> ! 	(flymake-split-string-remove-empty-edges str pattern)
> !       (flymake-split-string-remove-empty-edges str pattern)))

Hmmm... isn't this just
(defalias 'flymake-split-string 'flymake-split-string-remove-empty-edges) ?

> ! (defsubst flymake-get-temp-dir()
> !     (if (featurep 'xemacs)
> ! 	(temp-directory)
> !       temporary-file-directory))

Why not (if (fboundp 'temp-directory) ...) ?

>   (defun flymake-popup-menu(pos menu-data)
> !     (if (featurep 'xemacs)
> ! 	(let* ((x-pos       (nth 0 (nth 0 pos)))
> ! 	       (y-pos       (nth 1 (nth 0 pos)))
> ! 	       (fake-event-props  '(button 1 x 1 y 1)))
> ! 	  (setq fake-event-props (plist-put fake-event-props 'x x-pos))
> ! 	  (setq fake-event-props (plist-put fake-event-props 'y y-pos))
> ! 	  (popup-menu (flymake-make-xemacs-menu menu-data) (make-event 'button-press fake-event-props))
> ! 	  )
> !       (x-popup-menu pos (flymake-make-emacs-menu menu-data))))

Since Emacs-21 has popup-menu, we should be able to throw away
flymake-make-emacs-menu and rename flymake-make-xemacs-menu to
flymake-make-menu:
  
(defun flymake-popup-menu(pos menu-data)
  (popup-menu (flymake-make-menu menu-data)
              (if (not (featurep 'xemacs))
	          pos
                (let* ((x-pos       (nth 0 (nth 0 pos)))
	               (y-pos       (nth 1 (nth 0 pos)))
	               (fake-event-props  '(button 1 x 1 y 1)))
	          (setq fake-event-props (plist-put fake-event-props 'x x-pos))
	          (setq fake-event-props (plist-put fake-event-props 'y y-pos))
	          (make-event 'button-press fake-event-props)))))

>   (defun flymake-current-row()
>       "return current row in current frame"
> !     (if (featurep 'xemacs)
> ! 	(count-lines (window-start) (point))
> !       (+ (car (cdr (window-edges))) (count-lines (window-start) (point)))))

I suspect this should be:

(defun flymake-current-row()
  "return current row in current frame"
  (+ (count-lines (window-start) (point))
     (if (fboundp 'window-edges)
         (car (cdr (window-edges)))
       ;; On XEmacs we should probably use something else, but what??
       0)))

> ! (defsubst flymake-selected-frame()
> !     (if (featurep 'xemacs)
> !         (selected-window)
> !       (selected-frame)))

XEmacs has `selected-frame' as well, so the above code looks odd.
I must be missing something.
  

        Stefan

Re: Optimizations for flymake

[Kim F. Storm]

Stefan <monnier@iro.umontreal.ca> writes:

>> ! (defsubst flymake-makehash(&optional test)
>> !   (if (featurep 'xemacs)
>> !       (if test (make-hash-table :test test) (make-hash-table))
>> !     (makehash test)))
>
> Why not (if (fboundp 'make-hash-table) ...) ?

Right, that's better.

>   
>>   (defun flymake-float-time()
>> !     (if (featurep 'xemacs)
>> ! 	(let ((tm (current-time)))
>> ! 	  (multiple-value-bind (s0 s1 s2) (current-time)
>> ! 			       (+ (* (float (ash 1 16)) s0) (float s1) (* 0.0000001 s2))))
>> !       (float-time)))
>
> Why not (if (fboundp 'float-time) ...) ?
>

Because Emacs doesn't have multiple-value-bind.

>> ! (defsubst flymake-replace-regexp-in-string(regexp rep str)
>> !     (if (featurep 'xemacs)
>> ! 	(replace-in-string str regexp rep)
>> !       (replace-regexp-in-string regexp rep str)))
>
> Why not (if (fboundp 'replace-regexp-in-string) ...) ?
> I'd actually make it even more efficient than `defsubst':

Like this?

(defalias 'flymake-replace-regexp-in-string
  (if (fboundp 'replace-regexp-in-string))
      'replace-regexp-in-string
    'replace-in-string))


>
>> ! (defsubst flymake-split-string(str pattern)
>> !     (if (featurep 'xemacs)
>> ! 	(flymake-split-string-remove-empty-edges str pattern)
>> !       (flymake-split-string-remove-empty-edges str pattern)))
>
> Hmmm... isn't this just
> (defalias 'flymake-split-string 'flymake-split-string-remove-empty-edges) ?

Yes -- looks odd.

>
>> ! (defsubst flymake-get-temp-dir()
>> !     (if (featurep 'xemacs)
>> ! 	(temp-directory)
>> !       temporary-file-directory))
>
> Why not (if (fboundp 'temp-directory) ...) ?

Would work as well -- but it would hide why we make the check...

>
>>   (defun flymake-popup-menu(pos menu-data)
>> !     (if (featurep 'xemacs)
>> ! 	(let* ((x-pos       (nth 0 (nth 0 pos)))
>> ! 	       (y-pos       (nth 1 (nth 0 pos)))
>> ! 	       (fake-event-props  '(button 1 x 1 y 1)))
>> ! 	  (setq fake-event-props (plist-put fake-event-props 'x x-pos))
>> ! 	  (setq fake-event-props (plist-put fake-event-props 'y y-pos))
>> ! 	  (popup-menu (flymake-make-xemacs-menu menu-data) (make-event 'button-press fake-event-props))
>> ! 	  )
>> !       (x-popup-menu pos (flymake-make-emacs-menu menu-data))))
>
> Since Emacs-21 has popup-menu, we should be able to throw away
> flymake-make-emacs-menu and rename flymake-make-xemacs-menu to
> flymake-make-menu:
>   
> (defun flymake-popup-menu(pos menu-data)
>   (popup-menu (flymake-make-menu menu-data)
>               (if (not (featurep 'xemacs))
> 	          pos
>                 (let* ((x-pos       (nth 0 (nth 0 pos)))
> 	               (y-pos       (nth 1 (nth 0 pos)))
> 	               (fake-event-props  '(button 1 x 1 y 1)))
> 	          (setq fake-event-props (plist-put fake-event-props 'x x-pos))
> 	          (setq fake-event-props (plist-put fake-event-props 'y y-pos))
> 	          (make-event 'button-press fake-event-props)))))

I take your word for it :-)

>
>>   (defun flymake-current-row()
>>       "return current row in current frame"
>> !     (if (featurep 'xemacs)
>> ! 	(count-lines (window-start) (point))
>> !       (+ (car (cdr (window-edges))) (count-lines (window-start) (point)))))
>
> I suspect this should be:
>
> (defun flymake-current-row()
>   "return current row in current frame"
>   (+ (count-lines (window-start) (point))
>      (if (fboundp 'window-edges)
>          (car (cdr (window-edges)))
>        ;; On XEmacs we should probably use something else, but what??
>        0)))

Ok.

>
>> ! (defsubst flymake-selected-frame()
>> !     (if (featurep 'xemacs)
>> !         (selected-window)
>> !       (selected-frame)))
>
> XEmacs has `selected-frame' as well, so the above code looks odd.
> I must be missing something.

Me too.


It also occurred to me that rather than all the autoloads for the
various overlay things, it should simply do:

(require 'overlay)

Emacs has (featurep 'overlay) => t, so there's no need to
condition it with (featurep 'xemacs) either.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Optimizations for flymake

[David Kastrup]

storm@cua.dk (Kim F. Storm) writes:

> Stefan <monnier@iro.umontreal.ca> writes:
>
>>> ! (defsubst flymake-makehash(&optional test)
>>> !   (if (featurep 'xemacs)
>>> !       (if test (make-hash-table :test test) (make-hash-table))
>>> !     (makehash test)))
>>
>> Why not (if (fboundp 'make-hash-table) ...) ?
>
> Right, that's better.

makehash is a compiled Lisp function in `subr'.
(makehash &optional TEST)

This function is obsolete since 21.4;
use `make-hash-table' instead.

Not documented.

[back]

In short, just rip out the test and always use make-hash-table.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Optimizations for flymake

[Stefan]

> Because Emacs doesn't have multiple-value-bind.

It sure does.  After doing (eval-when-compile (require 'cl)), that is.

>> I'd actually make it even more efficient than `defsubst':

Oops, I forgot to delete the above line.

> Like this?

> (defalias 'flymake-replace-regexp-in-string
>   (if (fboundp 'replace-regexp-in-string))
>       'replace-regexp-in-string
>     'replace-in-string))

No, the args aren't passed in the right order.

>>> ! (defsubst flymake-get-temp-dir()
>>> !     (if (featurep 'xemacs)
>>> ! 	(temp-directory)
>>> !       temporary-file-directory))
>> 
>> Why not (if (fboundp 'temp-directory) ...) ?

> Would work as well -- but it would hide why we make the check...

Hmm... then I don't understand.  Why do we want temp-directory for XEmacs
and temporary-file-directory for Emacs, other than because that's what is
available?
I.e. why would we not want to use temp-directory under Emacs if someone
defined it?

> It also occurred to me that rather than all the autoloads for the
> various overlay things, it should simply do:

> (require 'overlay)

Agreed.

> Emacs has (featurep 'overlay) => t, so there's no need to
> condition it with (featurep 'xemacs) either.

There's a need to wrap it with condition-case because on Emacs-20,
(featurep 'overlay) => nil.  If we don't care about older Emacsen, wrapping
is unnecessary of course.


        Stefan

Re: Optimizations for flymake

[Pavel Kobiakov]

--- David Kastrup <dak@gnu.org> wrote:
> storm@cua.dk (Kim F. Storm) writes:
> 
> > Stefan <monnier@iro.umontreal.ca> writes:
> >
> >>> ! (defsubst flymake-makehash(&optional test)
> >>> !   (if (featurep 'xemacs)
> >>> !       (if test (make-hash-table :test test)
> (make-hash-table))
> >>> !     (makehash test)))
> >>
> >> Why not (if (fboundp 'make-hash-table) ...) ?
> >
> > Right, that's better.
> 
> makehash is a compiled Lisp function in `subr'.
> (makehash &optional TEST)
> 
> This function is obsolete since 21.4;
> use `make-hash-table' instead.
> 
> Not documented.
> 
> [back]
> 
> In short, just rip out the test and always use
> make-hash-table.
> 
> -- 
> David Kastrup, Kriemhildstr. 15, 44793 Bochum
> 

Getting rid of makehash won't allow Flymake to run on
older versions of Emacs, so I think it's not a good
idea.

  Pasha.



		
__________________________________ 
Do you Yahoo!? 
Check out the new Yahoo! Front Page. 
www.yahoo.com 

Re: Optimizations for flymake

[David Kastrup]

Pavel Kobiakov <pk_at_work@yahoo.com> writes:

> --- David Kastrup <dak@gnu.org> wrote:
>> storm@cua.dk (Kim F. Storm) writes:
>> 
>> > Stefan <monnier@iro.umontreal.ca> writes:
>> >
>> >>> ! (defsubst flymake-makehash(&optional test)
>> >>> !   (if (featurep 'xemacs)
>> >>> !       (if test (make-hash-table :test test)
>> (make-hash-table))
>> >>> !     (makehash test)))
>> >>
>> >> Why not (if (fboundp 'make-hash-table) ...) ?
>> >
>> > Right, that's better.
>> 
>> makehash is a compiled Lisp function in `subr'.
>> (makehash &optional TEST)
>> 
>> This function is obsolete since 21.4;
>> use `make-hash-table' instead.
>> 
>> Not documented.
>> 
>> [back]
>> 
>> In short, just rip out the test and always use
>> make-hash-table.
>> 
>> -- 
>> David Kastrup, Kriemhildstr. 15, 44793 Bochum
>> 
>
> Getting rid of makehash won't allow Flymake to run on
> older versions of Emacs, so I think it's not a good
> idea.

;;; flymake.el -- a universal on-the-fly syntax checker

;; Copyright (C) 2003 Free Software Foundation

;; Author:  Pavel Kobiakov <pk_at_work@yahoo.com>
;; Maintainer: Pavel Kobiakov <pk_at_work@yahoo.com>
;; Version: 0.3
;; Keywords: c languages tools

;; This file is part of GNU Emacs.

I think that in the file distributed as part of GNU Emacs,
compatibility to versions of old GNU Emacs versions should not really
be too high priority.  Of course, it is your decision as maintainer.
I just want to note that Emacs 21.3 is the currently released version
of Emacs, and even that has

make-hash-table is a built-in function.
(make-hash-table &rest KEYWORD-ARGS)

Create and return a new hash table.
Arguments are specified as keyword/argument pairs.  The following
arguments are defined:

:test TEST -- TEST must be a symbol that specifies how to compare keys.
Default is `eql'.  Predefined are the tests `eq', `eql', and `equal'.
User-supplied test and hash functions can be specified via
`define-hash-table-test'.

[...]

I mean, just _what_ are you trying to be compatible with?  You are
using `when' and `unless' which can't be used in Emacs-20.7 anyway
without loading cl-macs or something.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

RE: Optimizations for flymake

[Drew Adams]

Minor correction: `when' and `unless' are available in Emacs-20.7 (at least
20.7.3). They are defined in subr.el. - Drew


-----Original Message-----David Kastrup
You are using `when' and `unless' which can't be used in Emacs-20.7 anyway
without loading cl-macs or something.

Re: Optimizations for flymake

[Richard Stallman]

    Getting rid of makehash won't allow Flymake to run on
    older versions of Emacs, so I think it's not a good
    idea.

It is not part of the design goals of Emacs that people should be able
to remove certain Lisp code from Emacs and run it with old Emacs
versions.  I would rather simplify the code.