bug#74349: 30.0.92; Visiting a file under c-ts-mode loads cc-mode

[Stefan Monnier]

>> Could you clarify why you think it's a problem?
> cc-mode is large; loading it brings in many other cc-* files, which

That's indeed an inconvenient, but we usually consider it minor (several
modes seem happy to (require 'cc-mode) just to use one or two
variables from it such as syntax-tables).

> There's also the issue of the adverse consequences of loading cc-mode
> discussed in bug#74339.

Right, that's a self-inflicted problem because we did not follow our own
recommendation that loading a file should not affect the behavior
of Emacs.

> I hope those will be solved soon, but their current existence was the
> motivation for me to examine each and every reason for loading cc-mode
> when I didn't expect that to happen.

Fair enough.

>> It doesn't "funcall" it, but it does make sure the function is loaded:
>> 
>>               ;; If KEY is an extra parent it may remain not loaded
>>               ;; (hence with some of its mode-specific vars missing their
>>               ;; `safe-local-variable' property), leading to spurious
>>               ;; prompts about unsafe vars (bug#68246).
>>               (if (and (symbolp key) (autoloadp (indirect-function key)))
>>                   (ignore-errors (autoload-do-load (indirect-function key))))
>> 
>> > I didn't expect derived-mode-add-parents to cause c-mode be turned on,
>> 
>> It's not turned on: its file loaded.
>
> Why does it need to be loaded in this case?

As the comment explains: we load the mode to try and avoid spuriously
querying the user about "unsafe" settings.

> Is there some technical reason to do so in order to perform the
> settings of the variables defined by that mode?

Not to set the variable, but to check whether the setting is safe: If
your `.dir-locals.el` sets a variable FOO which is defined as
`safe-local-variable` in BASEMODE (e.g. `c-mode`) but you actually use
DERIVEDMODE (e.g. `c-ts-mode`) for that file and DERIVEDMODE does not
load BASEMODE, then we may have to load BASEMODE to discover that the
setting is safe.

The patch below should implement the refinement I suggested, where we
(auto)load BASENAME only in the case one of the vars doesn't yet have
a `safe-local-variable` property.

BTW, I notice that CC-mode's settings of `safe-local-variable` aren't
all autoloaded:

    % grep safe-local lisp/progmodes/cc*.el
    lisp/progmodes/cc-vars.el:;;;###autoload(put 'c-basic-offset 
'safe-local-variable 'integerp)
    lisp/progmodes/cc-vars.el:(put 'c-syntactic-indentation 
'safe-local-variable 'booleanp)
    lisp/progmodes/cc-vars.el:(put 'c-syntactic-indentation-in-macros 
'safe-local-variable 'booleanp)
    lisp/progmodes/cc-vars.el:;;;###autoload(put 'c-backslash-column 
'safe-local-variable 'integerp)
    lisp/progmodes/cc-vars.el:;;;###autoload (put 'c-font-lock-extra-types 
'safe-local-variable #'c-string-list-p)
    lisp/progmodes/cc-vars.el:;;;###autoload (put 'c++-font-lock-extra-types 
'safe-local-variable #'c-string-list-p)
    lisp/progmodes/cc-vars.el:;;;###autoload (put 'objc-font-lock-extra-types 
'safe-local-variable #'c-string-list-p)
    lisp/progmodes/cc-vars.el:;;;###autoload (put 'java-font-lock-extra-types 
'safe-local-variable #'c-string-list-p)
    lisp/progmodes/cc-vars.el:;;;###autoload (put 'idl-font-lock-extra-types 
'safe-local-variable #'c-string-list-p)
    lisp/progmodes/cc-vars.el:;;;###autoload (put 'pike-font-lock-extra-types 
'safe-local-variable #'c-string-list-p)
    lisp/progmodes/cc-vars.el:(put 'c-noise-macro-names 'safe-local-variable 
#'c-string-list-p)
    lisp/progmodes/cc-vars.el:(put 'c-noise-macro-with-parens-names 
'safe-local-variable #'c-string-list-p)
    lisp/progmodes/cc-vars.el:(put 'c-macro-names-with-semicolon 
'safe-local-variable
    lisp/progmodes/cc-vars.el:;;;###autoload(put 'c-file-style 
'safe-local-variable 'string-or-null-p)
    %

Alan, I assume this "inconsistency" is accidental (I "fixed" it in the
patch below).


        Stefan


diff --git a/lisp/files.el b/lisp/files.el
index bffdaa288a5..1bc2372eeec 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -4484,6 +4484,21 @@ dir-locals-collect-mode-variables
        ;; Need a new cons in case we setcdr later.
        (push (cons variable value) variables)))))
 
+(defun dir-locals--load-mode-if-needed (key alist)
+  ;; If KEY is an extra parent it may remain not loaded
+  ;; (hence with some of its mode-specific vars missing their
+  ;; `safe-local-variable' property), leading to spurious
+  ;; prompts about unsafe vars (bug#68246).
+ (when (and (symbolp key) (autoloadp (indirect-function key)))
+   (let ((unsafe nil))
+     (pcase-dolist (`(,var . ,_val) alist)
+       (unless (or (memq var '(mode eval))
+                   (get var 'safe-local-variable))
+        (setq unsafe t)))
+     (when unsafe
+       (ignore-errors
+        (autoload-do-load (indirect-function key)))))))
+
 (defun dir-locals-collect-variables (class-variables root variables
                                                      &optional predicate)
   "Collect entries from CLASS-VARIABLES into VARIABLES.
@@ -4514,15 +4514,9 @@
                   (funcall predicate key)
                 (or (not key)
                     (derived-mode-p key)))
-              ;; If KEY is an extra parent it may remain not loaded
-              ;; (hence with some of its mode-specific vars missing their
-              ;; `safe-local-variable' property), leading to spurious
-              ;; prompts about unsafe vars (bug#68246).
-              (if (and (symbolp key) (autoloadp (indirect-function key)))
-                  (ignore-errors (autoload-do-load (indirect-function key))))
               (let* ((alist (cdr entry))
                      (subdirs (assq 'subdirs alist)))
-                (if (or (not subdirs)
+                (when (or (not subdirs)
                         (progn
                           (setq alist (remq subdirs alist))
                           (cdr-safe subdirs))
@@ -4531,6 +4525,7 @@
                         ;; variables apply to this directory and N levels
                         ;; below it (0 == nil).
                         (equal root (expand-file-name default-directory)))
+                  (dir-locals--load-mode-if-needed key alist)
                     (setq variables (dir-locals-collect-mode-variables
                                      alist variables))))))))
       (error
diff --git a/lisp/progmodes/cc-vars.el b/lisp/progmodes/cc-vars.el
index f0e4c957ea5..305e324747d 100644
--- a/lisp/progmodes/cc-vars.el
+++ b/lisp/progmodes/cc-vars.el
@@ -530,7 +530,7 @@ c-syntactic-indentation
   :type 'boolean
   :group 'c)
 (make-variable-buffer-local 'c-syntactic-indentation)
-(put 'c-syntactic-indentation 'safe-local-variable 'booleanp)
+;;;###autoload(put 'c-syntactic-indentation 'safe-local-variable #'booleanp)
 
 (defcustom c-syntactic-indentation-in-macros t
   "Enable syntactic analysis inside macros.
@@ -550,7 +550,7 @@ c-syntactic-indentation-in-macros
   :type 'boolean
   :group 'c)
 
-(put 'c-syntactic-indentation-in-macros 'safe-local-variable 'booleanp)
+;;;###autoload(put 'c-syntactic-indentation-in-macros 'safe-local-variable 
#'booleanp)
 
 (defcustom c-defun-tactic 'go-outward
   "Whether functions are recognized inside, e.g., a class.
@@ -1732,7 +1732,7 @@ c-noise-macro-names
   :version "26.1"
   :type '(repeat :tag "List of names" string)
   :group 'c)
-(put 'c-noise-macro-names 'safe-local-variable #'c-string-list-p)
+;;;###autoload(put 'c-noise-macro-names 'safe-local-variable #'c-string-list-p)
 (make-variable-buffer-local 'c-noise-macro-names)
 
 (defcustom c-noise-macro-with-parens-names nil
@@ -1749,7 +1749,7 @@ c-noise-macro-with-parens-names
   :version "26.1"
   :type '(repeat :tag "List of names (possibly empty)" string)
   :group 'c)
-(put 'c-noise-macro-with-parens-names 'safe-local-variable #'c-string-list-p)
+;;;###autoload(put 'c-noise-macro-with-parens-names 'safe-local-variable 
#'c-string-list-p)
 (make-variable-buffer-local 'c-noise-macro-with-parens-names)
 
 (defun c-make-noise-macro-regexps ()
@@ -1798,8 +1798,7 @@ c-macro-names-with-semicolon
 `c-make-macros-with-semi-re' to set the necessary internal
 variables.")
 (make-variable-buffer-local 'c-macro-names-with-semicolon)
-(put 'c-macro-names-with-semicolon 'safe-local-variable
-     #'c-string-or-string-list-p)
+;;;###autoload(put 'c-macro-names-with-semicolon 'safe-local-variable 
#'c-string-or-string-list-p)
 
 (defun c-make-macro-with-semi-re ()
   ;; Convert `c-macro-names-with-semicolon' into the regexp