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

[Sebastian Wiesner <swiesner@lunaryorn.com>]

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