bug#19206: 25.0.50; CC Mode tracks wrong source files

[Sebastian Wiesner]

> Am 30.11.2014 um 19:42 schrieb Alan Mackenzie <acm@muc.de>:
> 
> Hello, again, Sebastian.
> 
> On Fri, Nov 28, 2014 at 10:25:42PM -0000, Alan Mackenzie wrote:
>> Hello, Sebastian.
>> In article <mailman.14863.1417170074.1147.bug-gnu-emacs@gnu.org> you wrote:
>>> CC Mode tracks wrong source files when a CC Mode derived mode is
>>> installed non-interactively.
> 
>>> To reproduce, save the following code as `cc-miscompile.el'
> 
>>> (require 'package)
>>> (require 'cc-defs)
> 
>>> (defun main ()
>>> (add-to-list 'package-archives '("melpa" . "http://melpa.org/packages/";))
> 
>>> (setq package-user-dir (make-temp-file "cc-miscompile" 'directory))
> 
>>> (package-initialize)
>>> (package-refresh-contents)
>>> (package-install 'd-mode)
> 
>>> (require 'd-mode)
> 
>>> (let ((source (get (intern "c-typedef-decl-kwds" c-lang-constants) 
>>> 'source)))
>>>   (message "Sources: %S" (mapcar 'car source)))
> 
>>> (delete-directory package-user-dir 'recursive))
> 
>>> (main)
> 
>>> and run it with `emacs -Q --script cc-miscompile.el'.  The output is as
>>> follows (package.el output shortened for readility):
> 
>>> Contacting host: melpa.org:80
>>> Contacting host: elpa.gnu.org:80
>>> [?]
>>> Sources: (d-mode cc-miscompile cc-langs)
> 
>>> Note that `cc-miscompile' ends up in the source list of
>>> `c-typedef-decl-kwds', even though it never actually calls any `c-*'
>>> functions at all.
> 
> OK.  The problem was that CC Mode was using the flag `load-in-progress'
> to assume that a CC Mode file was being loaded, for example by a
> `require' form inside a compilation of another CC Mode file.  This
> assumption breaks down when another file, such as cc-miscompile.el,
> while loading, initiates compilation of a CC Mode derivative.
> 
>> The byte compilation of d-mode.el is being done during the loading of
>> cc-miscompile.el.  This somewhat unusual constellation, I think, is
>> causing the problem.  When CC Mode determines the file name to put onto
>> a c-lang-defconst's 'source property, it gives priority to the load file
>> name, and only when this is nil does it use the byte-compile file name.
>> (This is in defsubst c-get-current-file in cc-defs.el).  It would seem
>> this is not the correct priority.
> 
>> I think swapping the first two arms of the `cond' form in
>> c-get-current-file may solve the problem.  It's a bit late to try this
>> tonight, I'll try it tomorrow.
> 
> No, that wouldn't work.  What I've implemented is when both loading and
> byte-compilation are active at the same time, CC Mode walks down the
> lisp stack to discover which one of them is actually active.

Well, if you say… I find this “solution” horrifying, but I am probably just 
unable to appreciate the full complexity of this issue.

>>> Naturally, CC Mode will later try to load this file, and fail if it is
>>> not in the `load-path'.  This effectively breaks installations of D
>>> Mode from non-interactive Emacs sessions.
> 
> Please try the following patch, which seems to work in the test case you
> supplied, in the real situation, and let me know how well it works.  The
> d-mode.elc produced can be successfully loaded into the Emacs that built
> it.

Do I need to build a patched Emacs to try it?

> diff --git a/lisp/progmodes/cc-bytecomp.el b/lisp/progmodes/cc-bytecomp.el
> index 1936627..bd2fd34 100644
> --- a/lisp/progmodes/cc-bytecomp.el
> +++ b/lisp/progmodes/cc-bytecomp.el
> @@ -84,13 +84,60 @@
>   ;;`(message ,@args)
>   )
> 
> +(defun cc-bytecomp-compiling-or-loading ()
> +  ;; Return whether byte-compilation or loading is currently active,
> +  ;; returning 'compiling or 'loading or nil.
> +  ;; If both are active, the "innermost" activity counts.  Note that
> +  ;; compilation can trigger loading (various `require' type forms)
> +  ;; and loading can trigger compilation (the package manager does
> +  ;; this).  We walk the lisp stack if necessary.
> +  (cond
> +   ((and load-in-progress
> +      (boundp 'byte-compile-dest-file)
> +      (stringp byte-compile-dest-file))
> +    (let ((n 0) elt)
> +      (while (and
> +           (setq elt (backtrace-frame n))
> +           (not (and (car elt)
> +                     (memq (cadr elt)
> +                           '(load byte-compile-file
> +                                  byte-recompile-directory
> +                                  batch-byte-compile)))))
> +     (setq n (1+ n)))
> +      (cond
> +       ((eq (cadr elt) 'load)
> +     'loading)
> +       ((memq (cadr elt) '(byte-compile-file
> +                        byte-recompile-directory
> +                        batch-byte-compile))
> +     'compiling)
> +       (t                            ; Can't happen.
> +     (message "cc-bytecomp-compiling-or-loading: System flags spuriously 
> set")
> +     nil))))
> +   (load-in-progress
> +    ;; Being loaded.
> +    'loading)
> +   ((and (boundp 'byte-compile-dest-file)
> +      (stringp byte-compile-dest-file))
> +    ;; Being compiled.
> +    'compiling)
> +   (t
> +    ;; Being evaluated interactively.
> +    nil)))
> +
> +(defsubst cc-bytecomp-is-compiling ()
> +  "Return non-nil if eval'ed during compilation."
> +  (eq (cc-bytecomp-compiling-or-loading) 'compiling))
> +
> +(defsubst cc-bytecomp-is-loading ()
> +  "Return non-nil if eval'ed during loading.
> +Nil will be returned if we're in a compilation triggered by the loading."
> +  (eq (cc-bytecomp-compiling-or-loading) 'loading))
> +
> (defun cc-bytecomp-setup-environment ()
>   ;; Eval'ed during compilation to setup variables, functions etc
>   ;; declared with `cc-bytecomp-defvar' et al.
> -  (if (not load-in-progress)
> -      ;; Look at `load-in-progress' to tell whether we're called
> -      ;; directly in the file being compiled or just from some file
> -      ;; being loaded during compilation.
> +  (if (not (cc-bytecomp-is-loading))
>       (let (p)
>       (if cc-bytecomp-environment-set
>           (error "Byte compilation environment already set - \
> @@ -138,7 +185,7 @@ perhaps a `cc-bytecomp-restore-environment' is forgotten 
> somewhere"))
> (defun cc-bytecomp-restore-environment ()
>   ;; Eval'ed during compilation to restore variables, functions etc
>   ;; declared with `cc-bytecomp-defvar' et al.
> -  (if (not load-in-progress)
> +  (if (not (cc-bytecomp-is-loading))
>       (let (p)
>       (setq p cc-bytecomp-unbound-variables)
>       (while p
> @@ -282,9 +329,7 @@ use within `eval-when-compile'."
>   `(eval-when-compile
>      (if (and (fboundp 'cc-bytecomp-is-compiling)
>             (cc-bytecomp-is-compiling))
> -      (if (or (not load-in-progress)
> -              (not (featurep ,cc-part)))
> -          (cc-bytecomp-load (symbol-name ,cc-part)))
> +          (cc-bytecomp-load (symbol-name ,cc-part))
>        (require ,cc-part))))
> 
> (defmacro cc-external-require (feature)
> @@ -296,12 +341,6 @@ afterwards.  Don't use within `eval-when-compile'."
>      (require ,feature)
>      (eval-when-compile (cc-bytecomp-setup-environment))))
> 
> -(defun cc-bytecomp-is-compiling ()
> -  "Return non-nil if eval'ed during compilation.  Don't use outside
> -`eval-when-compile'."
> -  (and (boundp 'byte-compile-dest-file)
> -       (stringp byte-compile-dest-file)))
> -
> (defmacro cc-bytecomp-defvar (var)
>   "Binds the symbol as a variable during compilation of the file,
> to silence the byte compiler.  Don't use within `eval-when-compile'."
> @@ -315,8 +354,7 @@ to silence the byte compiler.  Don't use within 
> `eval-when-compile'."
>             "cc-bytecomp-defvar: Saving %s (as unbound)" ',var)
>            (setq cc-bytecomp-unbound-variables
>                  (cons ',var cc-bytecomp-unbound-variables))))
> -       (if (and (cc-bytecomp-is-compiling)
> -             (not load-in-progress))
> +       (if (cc-bytecomp-is-compiling)
>          (progn
>            (defvar ,var)
>            (set ',var (intern (concat "cc-bytecomp-ignore-var:"
> @@ -344,8 +382,7 @@ at compile time, e.g. for macros and inline functions."
>            (setq cc-bytecomp-original-functions
>                  (cons (list ',fun nil 'unbound)
>                        cc-bytecomp-original-functions))))
> -       (if (and (cc-bytecomp-is-compiling)
> -             (not load-in-progress))
> +       (if (cc-bytecomp-is-compiling)
>          (progn
>            (fset ',fun (intern (concat "cc-bytecomp-ignore-fun:"
>                                        (symbol-name ',fun))))
> diff --git a/lisp/progmodes/cc-defs.el b/lisp/progmodes/cc-defs.el
> index 1d8b8ab..b0e83f3 100644
> --- a/lisp/progmodes/cc-defs.el
> +++ b/lisp/progmodes/cc-defs.el
> @@ -1823,19 +1823,22 @@ system."
> 
> (defvar c-lang-const-expansion nil)
> 
> +;; Ugly hack to pull in the definition of `cc-bytecomp-compiling-or-loading`
> +;; from cc-bytecomp to make it available at loadtime.  This is the same
> +;; mechanism used in cc-mode.el for `c-populate-syntax-table'.
> +(defalias 'cc-bytecomp-compiling-or-loading
> +  (cc-eval-when-compile
> +    (let ((f (symbol-function 'cc-bytecomp-compiling-or-loading)))
> +      (if (byte-code-function-p f) f (byte-compile f)))))
> +
> (defsubst c-get-current-file ()
>   ;; Return the base name of the current file.
> -  (let ((file (cond
> -            (load-in-progress
> -             ;; Being loaded.
> -             load-file-name)
> -            ((and (boundp 'byte-compile-dest-file)
> -                  (stringp byte-compile-dest-file))
> -             ;; Being compiled.
> -             byte-compile-dest-file)
> -            (t
> -             ;; Being evaluated interactively.
> -             (buffer-file-name)))))
> +  (let* ((c-or-l (cc-bytecomp-compiling-or-loading))
> +      (file
> +       (cond
> +        ((eq c-or-l 'loading) load-file-name)
> +        ((eq c-or-l 'compiling) byte-compile-dest-file)
> +        ((null c-or-l) (buffer-file-name)))))
>     (and file
>        (file-name-sans-extension
>         (file-name-nondirectory file)))))
> @@ -2073,9 +2076,7 @@ quoted."
>         (if (or (eq c-lang-const-expansion 'call)
>                 (and (not c-lang-const-expansion)
>                      (not mode))
> -                load-in-progress
> -                (not (boundp 'byte-compile-dest-file))
> -                (not (stringp byte-compile-dest-file)))
> +             (not (cc-bytecomp-is-compiling)))
>             ;; Either a straight call is requested in the context, or
>             ;; we're in an "uncontrolled" context and got no language,
>             ;; or we're not being byte compiled so the compile time
> diff --git a/lisp/progmodes/cc-langs.el b/lisp/progmodes/cc-langs.el
> index 68b2d62..22d78b5 100644
> --- a/lisp/progmodes/cc-langs.el
> +++ b/lisp/progmodes/cc-langs.el
> @@ -3252,10 +3252,7 @@ function it returns is byte compiled with all the 
> evaluated results
> from the language constants.  Use the `c-init-language-vars' macro to
> accomplish that conveniently."
> 
> -  (if (and (not load-in-progress)
> -        (boundp 'byte-compile-dest-file)
> -        (stringp byte-compile-dest-file))
> -
> +  (if (cc-bytecomp-is-compiling)
>       ;; No need to byte compile this lambda since the byte compiler is
>       ;; smart enough to detect the `funcall' construct in the
>       ;; `c-init-language-vars' macro below and compile it all straight
> 
> 
> 
> -- 
> Alan Mackenzie (Nuremberg, Germany).