Subject: Re: bug#21465: [PATCH] CC-modes hierarchy

From: Stefan Monnier <[email-masked]>
Date: Fri, 09 Oct 2015 16:49:19 -0400

> Why?  What is the point of the change?  It introduces an extra layer onto
> the stack of major modes, but this extra layer seems to do no more than
> execute two forms, `(c-initialize-cc-mode t)' and `(setq abbrev-mode t)'.

You already have this layer, with c-mode-common-hook and
c-mode-base-map.  My patch just makes this layering more standard and
then takes advantage of it to share some code.

Here's another version of the patch with the following changes:
- Got rid of c-derivative-mode (which I still use at other places in my
  local patches, but for now, I won't push for it any more).
- Keep c-make-inherited-keymap for backward compatibility.
- Fix the "c-mode-hook is run twice" bug, using the same hack as is used
  by define-globalized-minor-mode.
The third point also makes the c-mode-common "extra layer" more useful
since that hack is added in there.

Any objections to this version?

        Stefan

It does the following:
- Move code common to all CC-mode major modes to a c-mode-common-mode function.
- Remove code that's redundant with what define-derived-mode does:
  - set local-map
  - set syntax-table
  - set keymap parent
  - run mode hooks
- Mark c-make-inherited-keymap and c-run-mode-hooks obsolete.
- Use inhibit-modification-hooks rather than binding
  *-change-functions to nil.

diff --git a/lisp/progmodes/cc-mode.el b/lisp/progmodes/cc-mode.el
index 1b6a233..c476117 100644
--- a/lisp/progmodes/cc-mode.el
+++ b/lisp/progmodes/cc-mode.el
@@ -71,6 +71,19 @@
 ;;
 ;;    http://lists.sourceforge.net/mailman/listinfo/cc-mode-announce
 
+;; Externally maintained major modes which use CC-mode's engine:
+;; - cuda-mode
+;; - csharp-mode (https://github.com/josteink/csharp-mode)
+;; - haxe-mode
+;; - d-mode
+;; - dart-mode
+;; - cc-php-js-cs.el
+;; - php-mode
+;; - yang-mode
+;; - math-mode (mathematica)
+;; - unrealscript-mode
+;; - groovy-mode
+
 ;;; Code:
 
 ;; For Emacs < 22.2.
@@ -215,6 +228,8 @@ control).  See \"cc-mode.el\" for more info."
      ;; incompatible
      (t (error "CC Mode is incompatible with this version of Emacs")))
     map))
+(make-obsolete 'c-make-inherited-keymap
+	       "Use make-sparse-keymap + (set-keymap-parent c-mode-base-map); or derive from c-mode-common" "25.1")
 
 (defun c-define-abbrev-table (name defs &optional doc)
   ;; Compatibility wrapper for `define-abbrev' which passes a non-nil
@@ -667,7 +682,7 @@ compatible with old code; callers should always specify it."
     (setq c-new-BEG (point-min))
     (setq c-new-END (point-max))
     (save-excursion
-      (let (before-change-functions after-change-functions)
+      (let ((inhibit-modification-hooks t))
 	(mapc (lambda (fn)
 		(funcall fn (point-min) (point-max)))
 	      c-get-state-before-change-functions)
@@ -835,6 +850,7 @@ Note that the style variables are always made local to the buffer."
   (if (cc-bytecomp-fboundp 'run-mode-hooks)
       `(run-mode-hooks ,@hooks)
     `(progn ,@(mapcar (lambda (hook) `(run-hooks ,hook)) hooks))))
+(make-obsolete 'c-run-mode-hooks "derive your mode from c-common-mode" "25.1")
 
 
 ;;; Change hooks, linking with Font Lock and electric-indent-mode.
@@ -1219,7 +1235,7 @@ Note that the style variables are always made local to the buffer."
       (backward-char))			; back over (, [, <.
     (and (/= new-pos pos) new-pos)))
 
-(defun c-change-expand-fl-region (beg end old-len)
+(defun c-change-expand-fl-region (_beg _end _old-len)
   ;; Expand the region (c-new-BEG c-new-END) to an after-change font-lock
   ;; region.  This will usually be the smallest sequence of whole lines
   ;; containing `c-new-BEG' and `c-new-END', but if `c-new-BEG' is in a
@@ -1402,6 +1418,35 @@ This function is called from `c-common-init', once per mode initialization."
     (c-update-modeline)))
 
 
+(defvar c-mode-common-map c-mode-base-map)
+
+(defvar c-mode-common-update-mode-lines ()
+  "List of buffers where we should update the mode line.")
+
+(defun c-mode-common-update-mode-lines ()
+  (while c-mode-common-update-mode-lines
+    (let ((buf (pop c-mode-common-update-mode-lines)))
+      (when (buffer-live-p buf)
+	(with-current-buffer (c-update-modeline)))))
+  (remove-hook 'post-command-hook #'c-mode-common-update-mode-lines)
+  (remove-hook 'find-file-hook #'c-mode-common-update-mode-lines))
+
+(define-derived-mode c-mode-common prog-mode "CC-generic"
+  "Pseudo major mode, parent of all modes using the CC engine."
+  (c-initialize-cc-mode t)
+  (setq abbrev-mode t)  ;; Used for c-electric-continued-statement!
+  ;; We want to update the mode-line but *after* the major mode's hooks
+  ;; have been run.
+  ;; FIXME: Ideally, we'd just use a mode-line entry that's computed
+  ;; dynamically, but it's not clear how to do that (we could use
+  ;; `mode-line-process' but that would look weird if the user has decided to
+  ;; put mode-line-process elsewhere than immediately to the right of the major
+  ;; mode's name), so for now we'll use the ugly hack below, similar to the one
+  ;; used by `define-globalized-minor-mode'.
+  (push (current-buffer) c-mode-common-update-mode-lines)
+  (add-hook 'post-command-hook #'c-mode-common-update-mode-lines)
+  (add-hook 'find-file-hook #'c-mode-common-update-mode-lines))
+
 ;; Support for C
 
 (defvar c-mode-syntax-table
@@ -1414,7 +1459,7 @@ This function is called from `c-common-init', once per mode initialization."
   "Abbreviation table used in c-mode buffers.")
 
 (defvar c-mode-map
-  (let ((map (c-make-inherited-keymap)))
+  (let ((map (make-sparse-keymap)))
     ;; Add bindings which are only useful for C.
     (define-key map "\C-c\C-e"  'c-macro-expand)
     map)
@@ -1455,7 +1500,7 @@ This function is called from `c-common-init', once per mode initialization."
 (unless (fboundp 'prog-mode) (defalias 'prog-mode 'fundamental-mode))
 
 ;;;###autoload
-(define-derived-mode c-mode prog-mode "C"
+(define-derived-mode c-mode c-mode-common "C"
   "Major mode for editing C code.
 
 To submit a problem report, enter `\\[c-submit-bug-report]' from a
@@ -1470,18 +1515,11 @@ initialization, then `c-mode-hook'.
 
 Key bindings:
 \\{c-mode-map}"
-  (c-initialize-cc-mode t)
-  (set-syntax-table c-mode-syntax-table)
-  (setq local-abbrev-table c-mode-abbrev-table
-	abbrev-mode t)
-  (use-local-map c-mode-map)
   (c-init-language-vars-for 'c-mode)
   (c-make-macro-with-semi-re) ; matches macro names whose expansion ends with ;
   (c-common-init 'c-mode)
   (easy-menu-add c-c-menu)
-  (cc-imenu-init cc-imenu-c-generic-expression)
-  (c-run-mode-hooks 'c-mode-common-hook 'c-mode-hook)
-  (c-update-modeline))
+  (cc-imenu-init cc-imenu-c-generic-expression))
 
 
 ;; Support for C++
@@ -1497,7 +1535,7 @@ Key bindings:
   "Abbreviation table used in c++-mode buffers.")
 
 (defvar c++-mode-map
-  (let ((map (c-make-inherited-keymap)))
+  (let ((map (make-sparse-keymap)))
     ;; Add bindings which are only useful for C++.
     (define-key map "\C-c\C-e" 'c-macro-expand)
     (define-key map "\C-c:"    'c-scope-operator)
@@ -1510,7 +1548,7 @@ Key bindings:
 		  (cons "C++" (c-lang-const c-mode-menu c++)))
 
 ;;;###autoload
-(define-derived-mode c++-mode prog-mode "C++"
+(define-derived-mode c++-mode c-mode-common "C++"
   "Major mode for editing C++ code.
 To submit a problem report, enter `\\[c-submit-bug-report]' from a
 c++-mode buffer.  This automatically sets up a mail buffer with
@@ -1525,18 +1563,11 @@ initialization, then `c++-mode-hook'.
 
 Key bindings:
 \\{c++-mode-map}"
-  (c-initialize-cc-mode t)
-  (set-syntax-table c++-mode-syntax-table)
-  (setq local-abbrev-table c++-mode-abbrev-table
-	abbrev-mode t)
-  (use-local-map c++-mode-map)
   (c-init-language-vars-for 'c++-mode)
   (c-make-macro-with-semi-re) ; matches macro names whose expansion ends with ;
   (c-common-init 'c++-mode)
   (easy-menu-add c-c++-menu)
-  (cc-imenu-init cc-imenu-c++-generic-expression)
-  (c-run-mode-hooks 'c-mode-common-hook 'c++-mode-hook)
-  (c-update-modeline))
+  (cc-imenu-init cc-imenu-c++-generic-expression))
 
 
 ;; Support for Objective-C
@@ -1551,7 +1582,7 @@ Key bindings:
   "Abbreviation table used in objc-mode buffers.")
 
 (defvar objc-mode-map
-  (let ((map (c-make-inherited-keymap)))
+  (let ((map (make-sparse-keymap)))
     ;; Add bindings which are only useful for Objective-C.
     (define-key map "\C-c\C-e" 'c-macro-expand)
     map)
@@ -1563,7 +1594,7 @@ Key bindings:
 ;;;###autoload (add-to-list 'auto-mode-alist '("\\.m\\'" . objc-mode))
 
 ;;;###autoload
-(define-derived-mode objc-mode prog-mode "ObjC"
+(define-derived-mode objc-mode c-derivative-mode "ObjC"
   "Major mode for editing Objective C code.
 To submit a problem report, enter `\\[c-submit-bug-report]' from an
 objc-mode buffer.  This automatically sets up a mail buffer with
@@ -1578,18 +1609,11 @@ initialization, then `objc-mode-hook'.
 
 Key bindings:
 \\{objc-mode-map}"
-  (c-initialize-cc-mode t)
-  (set-syntax-table objc-mode-syntax-table)
-  (setq local-abbrev-table objc-mode-abbrev-table
-	abbrev-mode t)
-  (use-local-map objc-mode-map)
   (c-init-language-vars-for 'objc-mode)
   (c-make-macro-with-semi-re) ; matches macro names whose expansion ends with ;
   (c-common-init 'objc-mode)
   (easy-menu-add c-objc-menu)
-  (cc-imenu-init nil 'cc-imenu-objc-function)
-  (c-run-mode-hooks 'c-mode-common-hook 'objc-mode-hook)
-  (c-update-modeline))
+  (cc-imenu-init nil 'cc-imenu-objc-function))
 
 
 ;; Support for Java
@@ -1606,7 +1630,7 @@ Key bindings:
   "Abbreviation table used in java-mode buffers.")
 
 (defvar java-mode-map
-  (let ((map (c-make-inherited-keymap)))
+  (let ((map (make-sparse-keymap)))
     ;; Add bindings which are only useful for Java.
     map)
   "Keymap used in java-mode buffers.")
@@ -1624,7 +1648,7 @@ Key bindings:
 ;;;###autoload (add-to-list 'auto-mode-alist '("\\.java\\'" . java-mode))
 
 ;;;###autoload
-(define-derived-mode java-mode prog-mode "Java"
+(define-derived-mode java-mode c-mode-common "Java"
   "Major mode for editing Java code.
 To submit a problem report, enter `\\[c-submit-bug-report]' from a
 java-mode buffer.  This automatically sets up a mail buffer with
@@ -1639,17 +1663,10 @@ initialization, then `java-mode-hook'.
 
 Key bindings:
 \\{java-mode-map}"
-  (c-initialize-cc-mode t)
-  (set-syntax-table java-mode-syntax-table)
-  (setq local-abbrev-table java-mode-abbrev-table
-	abbrev-mode t)
-  (use-local-map java-mode-map)
   (c-init-language-vars-for 'java-mode)
   (c-common-init 'java-mode)
   (easy-menu-add c-java-menu)
-  (cc-imenu-init cc-imenu-java-generic-expression)
-  (c-run-mode-hooks 'c-mode-common-hook 'java-mode-hook)
-  (c-update-modeline))
+  (cc-imenu-init cc-imenu-java-generic-expression))
 
 
 ;; Support for CORBA's IDL language
@@ -1662,7 +1679,7 @@ Key bindings:
   "Abbreviation table used in idl-mode buffers.")
 
 (defvar idl-mode-map
-  (let ((map (c-make-inherited-keymap)))
+  (let ((map (make-sparse-keymap)))
     ;; Add bindings which are only useful for IDL.
     map)
   "Keymap used in idl-mode buffers.")
@@ -1673,7 +1690,7 @@ Key bindings:
 ;;;###autoload (add-to-list 'auto-mode-alist '("\\.idl\\'" . idl-mode))
 
 ;;;###autoload
-(define-derived-mode idl-mode prog-mode "IDL"
+(define-derived-mode idl-mode c-mode-common "IDL"
   "Major mode for editing CORBA's IDL, PSDL and CIDL code.
 To submit a problem report, enter `\\[c-submit-bug-report]' from an
 idl-mode buffer.  This automatically sets up a mail buffer with
@@ -1688,16 +1705,13 @@ initialization, then `idl-mode-hook'.
 
 Key bindings:
 \\{idl-mode-map}"
-  (c-initialize-cc-mode t)
-  (set-syntax-table idl-mode-syntax-table)
-  (setq local-abbrev-table idl-mode-abbrev-table)
-  (use-local-map idl-mode-map)
+  ;; No c-electric-continued-statement here, so we don't need abbrev-mode.
+  (kill-local-variable 'abbrev-mode)
   (c-init-language-vars-for 'idl-mode)
   (c-common-init 'idl-mode)
   (easy-menu-add c-idl-menu)
   ;;(cc-imenu-init cc-imenu-idl-generic-expression) ;TODO
-  (c-run-mode-hooks 'c-mode-common-hook 'idl-mode-hook)
-  (c-update-modeline))
+  )
 
 
 ;; Support for Pike
@@ -1712,7 +1726,7 @@ Key bindings:
   "Abbreviation table used in pike-mode buffers.")
 
 (defvar pike-mode-map
-  (let ((map (c-make-inherited-keymap)))
+  (let ((map (make-sparse-keymap)))
     ;; Additional bindings.
     (define-key map "\C-c\C-e" 'c-macro-expand)
     map)
@@ -1725,7 +1739,7 @@ Key bindings:
 ;;;###autoload (add-to-list 'interpreter-mode-alist '("pike" . pike-mode))
 
 ;;;###autoload
-(define-derived-mode pike-mode prog-mode "Pike"
+(define-derived-mode pike-mode c-mode-common "Pike"
   "Major mode for editing Pike code.
 To submit a problem report, enter `\\[c-submit-bug-report]' from a
 pike-mode buffer.  This automatically sets up a mail buffer with
@@ -1740,17 +1754,11 @@ initialization, then `pike-mode-hook'.
 
 Key bindings:
 \\{pike-mode-map}"
-  (c-initialize-cc-mode t)
-  (set-syntax-table pike-mode-syntax-table)
-  (setq local-abbrev-table pike-mode-abbrev-table
-	abbrev-mode t)
-  (use-local-map pike-mode-map)
   (c-init-language-vars-for 'pike-mode)
   (c-common-init 'pike-mode)
   (easy-menu-add c-pike-menu)
   ;;(cc-imenu-init cc-imenu-pike-generic-expression) ;TODO
-  (c-run-mode-hooks 'c-mode-common-hook 'pike-mode-hook)
-  (c-update-modeline))
+  )
 
 
 ;; Support for AWK
@@ -1767,11 +1775,11 @@ Key bindings:
   "Abbreviation table used in awk-mode buffers.")
 
 (defvar awk-mode-map
-  (let ((map (c-make-inherited-keymap)))
+  (let ((map (make-sparse-keymap)))
     ;; Add bindings which are only useful for awk.
-    (define-key map "#" 'self-insert-command)
-    (define-key map "/" 'self-insert-command)
-    (define-key map "*" 'self-insert-command)
+    (define-key map "#" 'self-insert-command);Override electric parent binding.
+    (define-key map "/" 'self-insert-command);Override electric parent binding.
+    (define-key map "*" 'self-insert-command);Override electric parent binding.
     (define-key map "\C-c\C-n" 'undefined) ; #if doesn't exist in awk.
     (define-key map "\C-c\C-p" 'undefined)
     (define-key map "\C-c\C-u" 'undefined)
@@ -1790,7 +1798,7 @@ Key bindings:
 (declare-function c-awk-unstick-NL-prop "cc-awk" ())
 
 ;;;###autoload
-(define-derived-mode awk-mode prog-mode "AWK"
+(define-derived-mode awk-mode c-mode-common "AWK"
   "Major mode for editing AWK code.
 To submit a problem report, enter `\\[c-submit-bug-report]' from an
 awk-mode buffer.  This automatically sets up a mail buffer with version
@@ -1809,17 +1817,9 @@ Key bindings:
   ;; declared in cc-awk.el and hasn't yet been loaded.
   :syntax-table nil
   (require 'cc-awk)			; Added 2003/6/10.
-  (c-initialize-cc-mode t)
-  (set-syntax-table awk-mode-syntax-table)
-  (setq local-abbrev-table awk-mode-abbrev-table
-	abbrev-mode t)
-  (use-local-map awk-mode-map)
   (c-init-language-vars-for 'awk-mode)
   (c-common-init 'awk-mode)
-  (c-awk-unstick-NL-prop)
-
-  (c-run-mode-hooks 'c-mode-common-hook 'awk-mode-hook)
-  (c-update-modeline))
+  (c-awk-unstick-NL-prop))
 
 
 ;; bug reporting
diff --git a/lisp/progmodes/cc-defs.el b/lisp/progmodes/cc-defs.el
index 6bd5815..f4b54e3 100644
--- a/lisp/progmodes/cc-defs.el
+++ b/lisp/progmodes/cc-defs.el
@@ -510,7 +510,7 @@ wouldn't be able to undo them.
 The return value is the value of the last form in BODY."
   `(let* ((modified (buffer-modified-p)) (buffer-undo-list t)
 	  (inhibit-read-only t) (inhibit-point-motion-hooks t)
-	  before-change-functions after-change-functions
+	  (inhibit-modification-hooks t)
 	  deactivate-mark
 	  buffer-file-name buffer-file-truename ; Prevent primitives checking
 						; for file modification