all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#74349: 30.0.92; Visiting a file under c-ts-mode loads cc-mode
@ 2024-11-13 19:24 Eli Zaretskii
  2024-11-13 20:42 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 7+ messages in thread
From: Eli Zaretskii @ 2024-11-13 19:24 UTC (permalink / raw)
  To: 74349; +Cc: Stefan Monnier

To reproduce:

  emacs -Q
  M-: (featurep 'cc-mode) RET
   => nil
  M-x load-library RET c-ts-mode RET
  M-: (featurep 'cc-mode) RET
   => nil
  C-x C-f src/dispnew.c RET
  M-: major-mode RET
   => c-ts-mode
  M-: (featurep 'cc-mode) RET
   => t
  M-: c-noise-macro-names RET
   => ("INLINE" "NO_INLINE" "ATTRIBUTE_NO_SANITIZE_UNDEFINED" "UNINIT" "CALLBACK" "ALIGN_STACK" "ATTRIBUTE_MALLOC" "ATTRIBUTE_DEALLOC_FREE" "ANDROID_EXPORT" "TEST_STATIC" "INLINE_HEADER_BEGIN" "INLINE_HEADER_END")

So the C file was correctly visited using c-ts-mode, but cc-mode was
still loaded, quite unexpectedly.  Moreover, the settings in our
.dir-locals.el that are defined for c-mode were applied to the buffer
which is not under c-mode.

AFAICT, this happens because of three factors:

  . Loading c-ts-mode evaluates this:

     (derived-mode-add-parents 'c-ts-mode '(c-mode))

  . We have in our .dir-locals.el settings for c-mode:

     (c-mode . ((c-file-style . "GNU")
		(c-noise-macro-names . ("INLINE" "NO_INLINE" "ATTRIBUTE_NO_SANITIZE_UNDEFINED"
					"UNINIT" "CALLBACK" "ALIGN_STACK" "ATTRIBUTE_MALLOC"
					"ATTRIBUTE_DEALLOC_FREE" "ANDROID_EXPORT" "TEST_STATIC"
					"INLINE_HEADER_BEGIN" "INLINE_HEADER_END"))
		(electric-quote-comment . nil)
		(electric-quote-string . nil)
		(indent-tabs-mode . t)
		(mode . bug-reference-prog)))

  . hack-dir-local-variables 'funcall's the mode symbol when
    processing directory-local variables for that mode

A C and Lisp backtrace showing that cc-mode was loaded as side effect
of processing .dir-locals.el is shown below.  I could understand why a
mode is funcall'ed when it is being turned on, but I didn't expect
derived-mode-add-parents to cause c-mode be turned on, and its
directory-local settings be applied, when c-ts-mode is used to visit a
file.  That's at least unexpected, and arguably a bug.

Can this be avoided, please?  Users who want to use c-ts-mode for C
files will not necessarily be happy to have cc-mode and all its
dependencies loaded, and the c-mode-related settings applied, just
because of unrelated c-mode settings in .dir-locals.el.  It bloats the
memory footprint of Emacs for no good reason, and it modifies
variables the user didn't mean to touch.

Here's the backtrace.  I obtained it by setting a breakpoint in Fload
and waiting for cc-mode to be loaded.

  (gdb) break Fload
  Breakpoint 3 at 0x84b569: file lread.c, line 1326.
  (gdb) commands
  Type commands for breakpoint(s) 3, one per line.
  End with a line saying just "end".
  >pp file
  >end
  (gdb) c
  Continuing.

  Thread 1 hit Breakpoint 3, Fload (file=XIL(0x80000000094bf9d8),
      noerror=XIL(0), nomessage=XIL(0x30), nosuffix=XIL(0),
      must_suffix=XIL(0x30)) at lread.c:1326
  1326      file_stream stream UNINIT;
  "cc-mode"
  (gdb) bt
  #0  Fload (file=XIL(0x80000000094bf9d8), noerror=XIL(0), nomessage=XIL(0x30),
      nosuffix=XIL(0), must_suffix=XIL(0x30)) at lread.c:1326
  #1  0x0084d064 in save_match_data_load (file=XIL(0x80000000094bf9d8),
      noerror=XIL(0), nomessage=XIL(0x30), nosuffix=XIL(0),
      must_suffix=XIL(0x30)) at lread.c:1783
  #2  0x007fcf6d in load_with_autoload_queue (file=XIL(0x80000000094bf9d8),
      noerror=XIL(0), nomessage=XIL(0x30), nosuffix=XIL(0),
      must_suffix=XIL(0x30)) at eval.c:2382
  #3  0x007fd29b in Fautoload_do_load (fundef=XIL(0xc0000000094bf998),
      funname=XIL(0), macro_only=XIL(0)) at eval.c:2428
  #4  0x007fff77 in funcall_subr (subr=0xe5d100 <Sautoload_do_load>, numargs=1,
      args=0xa0916b8) at eval.c:3165
  #5  0x0086d77d in exec_byte_code (fun=XIL(0xa0000000094c03a8),
      args_template=513, nargs=1, args=0xa091768) at bytecode.c:812
  #6  0x008007f3 in funcall_lambda (fun=XIL(0xa00000000df14188), nargs=1,
      arg_vector=0xa091560) at eval.c:3252
  #7  0x007ff8b9 in funcall_general (fun=XIL(0xa00000000df14188), numargs=1,
      args=0xa091560) at eval.c:3044
  #8  0x007ffbe2 in Ffuncall (nargs=2, args=0xa091558) at eval.c:3093
  #9  0x007fef39 in run_hook_wrapped_funcall (nargs=2, args=0xa091558)
      at eval.c:2872
  #10 0x007ff2f2 in run_hook_with_args (nargs=2, args=0xa091558,
      funcall=0x7feef1 <run_hook_wrapped_funcall>) at eval.c:2953
  #11 0x007fef88 in Frun_hook_wrapped (nargs=2, args=0xa091558) at eval.c:2887
  #12 0x0080038b in funcall_subr (subr=0xe5d2c0 <Srun_hook_wrapped>, numargs=2,
      args=0xa091558) at eval.c:3184
  #13 0x0086d77d in exec_byte_code (fun=XIL(0xa000000009bc4278),
      args_template=769, nargs=1, args=0xa091560) at bytecode.c:812
  #14 0x008007f3 in funcall_lambda (fun=XIL(0xa000000009556de8), nargs=2,
      arg_vector=0x607e9f0) at eval.c:3252
  #15 0x007ff8b9 in funcall_general (fun=XIL(0xa000000009556de8), numargs=2,
      args=0x607e9f0) at eval.c:3044
  #16 0x007ffbe2 in Ffuncall (nargs=3, args=0x607e9e8) at eval.c:3093
  #17 0x007f099a in Ffuncall_interactively (nargs=3, args=0x607e9e8)
      at callint.c:250
  #18 0x0080038b in funcall_subr (subr=0xe5c840 <Sfuncall_interactively>,
      numargs=3, args=0x607e9e8) at eval.c:3184
  #19 0x007ff852 in funcall_general (fun=XIL(0xa000000000e5c840), numargs=3,
      args=0x607e9e8) at eval.c:3040
  #20 0x007ffbe2 in Ffuncall (nargs=4, args=0x607e9e0) at eval.c:3093
  #21 0x007fed45 in Fapply (nargs=3, args=0x607ebf8) at eval.c:2765
  #22 0x007f0fea in Fcall_interactively (function=XIL(0x85c18c8),
      record_flag=XIL(0), keys=XIL(0xa00000000b9f6798)) at callint.c:342
  #23 0x007fff77 in funcall_subr (subr=0xe5c880 <Scall_interactively>,
      numargs=3, args=0xa091078) at eval.c:3165
  #24 0x0086d77d in exec_byte_code (fun=XIL(0xa000000009bc6ba0),
      args_template=1025, nargs=1, args=0x607f7a0) at bytecode.c:812
  #25 0x008007f3 in funcall_lambda (fun=XIL(0xa000000009bc6ba0), nargs=1,
      arg_vector=0x607f798) at eval.c:3252
  #26 0x007ff8b9 in funcall_general (fun=XIL(0xa000000009bc6ba0), numargs=1,
      args=0x607f798) at eval.c:3044
  #27 0x007ffbe2 in Ffuncall (nargs=2, args=0x607f790) at eval.c:3093
  #28 0x00705721 in command_loop_1 () at keyboard.c:1550
  #29 0x007fa8d1 in internal_condition_case (bfun=0x704b31 <command_loop_1>,
      handlers=XIL(0x90), hfun=0x703b8a <cmd_error>) at eval.c:1613
  #30 0x00704596 in command_loop_2 (handlers=XIL(0x90)) at keyboard.c:1168
  #31 0x007f9956 in internal_catch (tag=XIL(0x12720),
      func=0x70455f <command_loop_2>, arg=XIL(0x90)) at eval.c:1292
  #32 0x00704501 in command_loop () at keyboard.c:1146
  #33 0x007035ea in recursive_edit_1 () at keyboard.c:754
  #34 0x00703888 in Frecursive_edit () at keyboard.c:837
  #35 0x006fe9a9 in main (argc=2, argv=0x7b225f8) at emacs.c:2635

  Lisp Backtrace:
  "autoload-do-load" (0xa0916b8)
  "dir-locals-collect-variables" (0xa091630)
  "hack-dir-local--get-variables" (0xa0915c0)
  0xdf14188 PVEC_CLOSURE
  "run-hook-wrapped" (0xa091558)
  "hack-dir-local-variables" (0xa0914d8)
  "hack-local-variables" (0xa091490)
  "run-mode-hooks" (0xa091448)
  "c-ts-mode" (0xa0913f8)
  "set-auto-mode-0" (0xa0913a8)
  "set-auto-mode--apply-alist" (0xa091328)
  "set-auto-mode" (0xa0912d0)
  "normal-mode" (0xa091280)
  "after-find-file" (0xa091220)
  "find-file-noselect-1" (0xa091190)
  "find-file-noselect" (0xa091108)
  "find-file" (0x607e9f0)
  "funcall-interactively" (0x607e9e8)
  "call-interactively" (0xa091078)
  "command-execute" (0x607f798)

In GNU Emacs 30.0.92 (build 22, i686-pc-mingw32) of 2024-11-13 built on
 ELIZ-PC
Windowing system distributor 'Microsoft Corp.', version 10.0.22631
System Description: Microsoft Windows 10 Enterprise (v10.0.2009.22631.4460)

Configured using:
 'configure -C --prefix=/d/usr --with-wide-int
 --enable-checking=yes,glyphs --without-native-compilation 'CFLAGS=-O0
 -gdwarf-4 -g3''

Configured features:
ACL GIF GMP GNUTLS HARFBUZZ JPEG LCMS2 LIBXML2 MODULES NOTIFY W32NOTIFY
PDUMPER PNG RSVG SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS
TREE_SITTER WEBP XPM ZLIB

Important settings:
  value of $LANG: ENU
  locale-coding-system: cp1252

Major mode: ELisp/l

Minor modes in effect:
  bug-reference-prog-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  minibuffer-regexp-mode: t
  line-number-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa epg rfc6068
epg-config gnus-util time-date subr-x mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils cl-extra shortdoc
text-property-search comp-common rx derived help-fns radix-tree
help-mode vc-git diff-mode track-changes easy-mmode vc-dispatcher
bug-reference byte-opt gv bytecomp byte-compile cc-mode cc-fonts
cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs
cl-seq c++-ts-mode c-ts-mode c-ts-common treesit cl-loaddefs cl-lib
thingatpt find-func rmc iso-transl tooltip cconv eldoc paren electric
uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
touch-screen dos-w32 ls-lisp disp-table term/w32-win w32-win w32-vars
term/common-win tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode register
page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads w32notify w32 lcms2 multi-tty move-toolbar make-network-process
emacs)

Memory information:
((conses 16 103247 13845) (symbols 48 10527 0) (strings 16 31840 3566)
 (string-bytes 1 855779) (vectors 16 16474)
 (vector-slots 8 171529 10546) (floats 8 88 4) (intervals 40 673 162)
 (buffers 896 13))





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

* bug#74349: 30.0.92; Visiting a file under c-ts-mode loads cc-mode
  2024-11-13 19:24 bug#74349: 30.0.92; Visiting a file under c-ts-mode loads cc-mode Eli Zaretskii
@ 2024-11-13 20:42 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-11-14  6:35   ` Eli Zaretskii
  0 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-11-13 20:42 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 74349

> So the C file was correctly visited using c-ts-mode, but cc-mode was
> still loaded, quite unexpectedly.

Could you clarify why you think it's a problem?

> Moreover, the settings in our .dir-locals.el that are defined for
> c-mode were applied to the buffer which is not under c-mode.

Could you similarly clarify why you think it's a problem?

>   . hack-dir-local-variables 'funcall's the mode symbol when
>     processing directory-local variables for that mode

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.

> and its directory-local settings be applied, when c-ts-mode is
> used to visit a file.

Looks like you forgot about it, but yes, that was discussed explicitly
when we discussed `derived-mode-add-parents`.  We decided back then that
the extra var-settings will likely be harmless.

> Can this be avoided, please?  Users who want to use c-ts-mode for C
> files will not necessarily be happy to have cc-mode and all its
> dependencies loaded, and the c-mode-related settings applied, just
> because of unrelated c-mode settings in .dir-locals.el.  It bloats the
> memory footprint of Emacs for no good reason,

Maybe we could try and add more tests in the above code before deciding
to `autoload-do-load`.  E.g. we could first check the list of variables
being set and if they all already have a `safe-local-variable` property,
then we don't need to autoload the mode (and then we'd have to make sure
CC-mode's `safe-local-variable` settings are all autoloaded).

> and it modifies variables the user didn't mean to touch.

We don't really know that without reading the user's mind, in the
general case.  Maybe the `c-mode` settings set only things like
`indent-tabs-mode` and `require-final-newline` and the users definitely
want them to apply to any mode used for the C files rather than only for
the `cc-c-mode`.


        Stefan






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

* bug#74349: 30.0.92; Visiting a file under c-ts-mode loads cc-mode
  2024-11-13 20:42 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-11-14  6:35   ` Eli Zaretskii
  2024-11-14 16:21     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 7+ messages in thread
From: Eli Zaretskii @ 2024-11-14  6:35 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 74349

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: 74349@debbugs.gnu.org
> Date: Wed, 13 Nov 2024 15:42:21 -0500
> 
> > So the C file was correctly visited using c-ts-mode, but cc-mode was
> > still loaded, quite unexpectedly.
> 
> Could you clarify why you think it's a problem?

cc-mode is large; loading it brings in many other cc-* files, which
basically in this scenario are just ballast.  In the
native-compilation case, this means loading quite a few additional
*.eln files which are not really used.  It's not "economical", and
looks unclean to me.

There's also the issue of the adverse consequences of loading cc-mode
discussed in bug#74339.  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.

> > Moreover, the settings in our .dir-locals.el that are defined for
> > c-mode were applied to the buffer which is not under c-mode.
> 
> Could you similarly clarify why you think it's a problem?

It was unexpected, that's all.

> >   . hack-dir-local-variables 'funcall's the mode symbol when
> >     processing directory-local variables for that mode
> 
> 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?  Is there some technical
reason to do so in order to perform the settings of the variables
defined by that mode?  Or is this just an artifact of the
implementation of hack-dir-local-variables, and thus could be avoided?

> > and its directory-local settings be applied, when c-ts-mode is
> > used to visit a file.
> 
> Looks like you forgot about it, but yes, that was discussed explicitly
> when we discussed `derived-mode-add-parents`.  We decided back then that
> the extra var-settings will likely be harmless.

I guess I did forget.  But the main issue for me here is that cc-mode
is loaded, not that the variables are defined.

> > Can this be avoided, please?  Users who want to use c-ts-mode for C
> > files will not necessarily be happy to have cc-mode and all its
> > dependencies loaded, and the c-mode-related settings applied, just
> > because of unrelated c-mode settings in .dir-locals.el.  It bloats the
> > memory footprint of Emacs for no good reason,
> 
> Maybe we could try and add more tests in the above code before deciding
> to `autoload-do-load`.  E.g. we could first check the list of variables
> being set and if they all already have a `safe-local-variable` property,
> then we don't need to autoload the mode (and then we'd have to make sure
> CC-mode's `safe-local-variable` settings are all autoloaded).

If that could be done, I think it would be a good thing, yes.

> > and it modifies variables the user didn't mean to touch.
> 
> We don't really know that without reading the user's mind, in the
> general case.  Maybe the `c-mode` settings set only things like
> `indent-tabs-mode` and `require-final-newline` and the users definitely
> want them to apply to any mode used for the C files rather than only for
> the `cc-c-mode`.

Right, I see the logic of setting the variables now.





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

* bug#74349: 30.0.92; Visiting a file under c-ts-mode loads cc-mode
  2024-11-14  6:35   ` Eli Zaretskii
@ 2024-11-14 16:21     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-11-23 12:32       ` Eli Zaretskii
  0 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-11-14 16:21 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 74349

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






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

* bug#74349: 30.0.92; Visiting a file under c-ts-mode loads cc-mode
  2024-11-14 16:21     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-11-23 12:32       ` Eli Zaretskii
  2024-11-24  3:13         ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 7+ messages in thread
From: Eli Zaretskii @ 2024-11-23 12:32 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 74349

> Cc: 74349@debbugs.gnu.org
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Thu, 14 Nov 2024 11:21:49 -0500
> 
> >> 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.

I think you should install this on master, thanks.

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

Alan, any comments?





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

* bug#74349: 30.0.92; Visiting a file under c-ts-mode loads cc-mode
  2024-11-23 12:32       ` Eli Zaretskii
@ 2024-11-24  3:13         ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-12-07 12:20           ` Eli Zaretskii
  0 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-11-24  3:13 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 74349

>> 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.
>
> I think you should install this on master, thanks.

Done, thanks.


        Stefan






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

* bug#74349: 30.0.92; Visiting a file under c-ts-mode loads cc-mode
  2024-11-24  3:13         ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-12-07 12:20           ` Eli Zaretskii
  0 siblings, 0 replies; 7+ messages in thread
From: Eli Zaretskii @ 2024-12-07 12:20 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 74349-done

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: 74349@debbugs.gnu.org
> Date: Sat, 23 Nov 2024 22:13:19 -0500
> 
> >> 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.
> >
> > I think you should install this on master, thanks.
> 
> Done, thanks.

No further comments, so I'm now closing this bug.





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

end of thread, other threads:[~2024-12-07 12:20 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-13 19:24 bug#74349: 30.0.92; Visiting a file under c-ts-mode loads cc-mode Eli Zaretskii
2024-11-13 20:42 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-11-14  6:35   ` Eli Zaretskii
2024-11-14 16:21     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-11-23 12:32       ` Eli Zaretskii
2024-11-24  3:13         ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-12-07 12:20           ` Eli Zaretskii

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.