bug#35454: 26.2.50; CC-Mode fontification fails inside macro

bug#35454: 26.2.50; CC-Mode fontification fails inside macro

[Mauro Aranda]

[-- Attachment #1: Type: text/plain, Size: 3818 bytes --]

Hello.

Steps to reproduce:
1) emacs -Q
2) Open up a new .c file:
C-x C-f test.c
3) Type this text:
#define FOO        \
  /* Some comms.  */     \
  struct foobar my_foo; \
  struct foobar my_bar;

The first struct foobar after the comment is not fontified as the second
one.  That is, the second foobar has face font-lock-type-face, and
my_bar has face font-lock-variable-name-face, but the first foobar and
my_foo don't get those face values.

I actually bumped into this issue while visiting the emacs source file
src/editfns.c.  In that file, search for "#define EXTRA_CONTEXT_FIELDS"
and the problem should be evident.

I can reproduce it with the latest Emacs 26, as well as with the latest
master:
Repository revision: 8dc00b2f1e6523c634df3e24379afbe712a32b27
Repository branch: master

Best regards,
Mauro.


In GNU Emacs 26.2.50 (build 1, i686-pc-linux-gnu, GTK+ Version 3.18.9)
 of 2019-04-23 built on the-blackbeard
Repository revision: 9ec18fbd560526ab19c6171aff15995d1307233e
Windowing system distributor 'The X.Org Foundation', version 11.0.11906000
System Description:    Ubuntu 16.04.6 LTS

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
(New file)
Auto-saving...done
Quit
Making completion list...

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS GLIB
NOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM THREADS LCMS2

Important settings:
  value of $LANG: en_US.utf8
  value of $XMODIFIERS:
  locale-coding-system: utf-8-unix

Major mode: C/*l

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-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
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t
  abbrev-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny seq byte-opt gv
bytecomp byte-compile cconv dired dired-loaddefs format-spec rfc822 mml
mml-sec password-cache epa derived epg epg-config gnus-util rmail
rmail-loaddefs mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils cc-mode cc-fonts easymenu cc-guess
cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs
cl-loaddefs cl-lib elec-pair time-date mule-util tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow isearch timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame cl-generic 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 charscript charprop case-table epa-hook jka-cmpr-hook
help simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote threads dbusbind inotify lcms2 dynamic-setting
system-font-setting font-render-setting move-toolbar gtk x-toolkit x
multi-tty make-network-process emacs)

Memory information:
((conses 8 115795 11061)
 (symbols 24 22635 1)
 (miscs 20 58 184)
 (strings 16 34126 1595)
 (string-bytes 1 1031562)
 (vectors 12 17262)
 (vector-slots 4 561150 8896)
 (floats 8 51 121)
 (intervals 28 306 68)
 (buffers 536 13)
 (heap 1024 38012 1009))

[-- Attachment #2: Type: text/html, Size: 4276 bytes --]

bug#35454: 26.2.50; CC-Mode fontification fails inside macro

[Alan Mackenzie]

Hello, Mauro.

On Sat, Apr 27, 2019 at 12:57:05 -0300, Mauro Aranda wrote:
> Hello.

> Steps to reproduce:
> 1) emacs -Q
> 2) Open up a new .c file:
> C-x C-f test.c
> 3) Type this text:
> #define FOO        \
>   /* Some comms.  */     \
>   struct foobar my_foo; \
>   struct foobar my_bar;

> The first struct foobar after the comment is not fontified as the second
> one.  That is, the second foobar has face font-lock-type-face, and
> my_bar has face font-lock-variable-name-face, but the first foobar and
> my_foo don't get those face values.

> I actually bumped into this issue while visiting the emacs source file
> src/editfns.c.  In that file, search for "#define EXTRA_CONTEXT_FIELDS"
> and the problem should be evident.

Thanks!  I can reproduce this easily, and will look into it in the next
day or two.

Of interest is the fact that if FOO is given an empty argument list
(i.e. one writes
    #define FOO()  \
    ...
), the bug doesn't happen.

> I can reproduce it with the latest Emacs 26, as well as with the latest
> master:
> Repository revision: 8dc00b2f1e6523c634df3e24379afbe712a32b27
> Repository branch: master

> Best regards,
> Mauro.

[ .... ]

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#35454: 26.2.50; CC-Mode fontification fails inside macro

[Alan Mackenzie]

Hello again, Mauro.

On Sat, Apr 27, 2019 at 20:36:46 +0000, Alan Mackenzie wrote:
> On Sat, Apr 27, 2019 at 12:57:05 -0300, Mauro Aranda wrote:
> > Hello.

> > Steps to reproduce:
> > 1) emacs -Q
> > 2) Open up a new .c file:
> > C-x C-f test.c
> > 3) Type this text:
> > #define FOO        \
> >   /* Some comms.  */     \
> >   struct foobar my_foo; \
> >   struct foobar my_bar;

> > The first struct foobar after the comment is not fontified as the second
> > one.  That is, the second foobar has face font-lock-type-face, and
> > my_bar has face font-lock-variable-name-face, but the first foobar and
> > my_foo don't get those face values.

> > I actually bumped into this issue while visiting the emacs source file
> > src/editfns.c.  In that file, search for "#define EXTRA_CONTEXT_FIELDS"
> > and the problem should be evident.

> Thanks!  I can reproduce this easily, and will look into it in the next
> day or two.

Please try out the patch below.  On my system, it corrects the
fontification in both your test file and editfns.c.

> Of interest is the fact that if FOO is given an empty argument list
> (i.e. one writes
>     #define FOO()  \
>     ...
> ), the bug doesn't happen.

> > I can reproduce it with the latest Emacs 26, as well as with the latest
> > master:
> > Repository revision: 8dc00b2f1e6523c634df3e24379afbe712a32b27
> > Repository branch: master



diff -r 9d58d1e3ab27 cc-engine.el
--- a/cc-engine.el	Sat Apr 27 17:07:23 2019 +0000
+++ b/cc-engine.el	Wed May 01 20:46:40 2019 +0000
@@ -5668,7 +5668,10 @@
 	       (setq cfd-re-match cfd-limit)
 	       nil)
 	      ((c-got-face-at
-		(if (setq cfd-re-match (match-end 1))
+		(if (setq cfd-re-match
+			  (or (match-end 1)
+			      (and c-dposr-cpp-macro-depth
+				   (match-end (1+ c-dposr-cpp-macro-depth)))))
 		    ;; Matched the end of a token preceding a decl spot.
 		    (progn
 		      (goto-char cfd-re-match)
@@ -5679,15 +5682,19 @@
 		c-literal-faces)
 	       ;; Pseudo match inside a comment or string literal.  Skip out
 	       ;; of comments and string literals.
-	       (while (progn
-			(unless
-			    (and (match-end 1)
-				 (c-got-face-at (1- (point)) c-literal-faces)
-				 (not (c-got-face-at (point) c-literal-faces)))
-			  (goto-char (c-next-single-property-change
-				      (point) 'face nil cfd-limit)))
-			(and (< (point) cfd-limit)
-			     (c-got-face-at (point) c-literal-faces))))
+	       (while
+		   (progn
+		     (unless
+			 (and
+			  (or (match-end 1)
+			      (and c-dposr-cpp-macro-depth
+				   (match-end (1+ c-dposr-cpp-macro-depth))))
+			  (c-got-face-at (1- (point)) c-literal-faces)
+			  (not (c-got-face-at (point) c-literal-faces)))
+		       (goto-char (c-next-single-property-change
+				   (point) 'face nil cfd-limit)))
+		     (and (< (point) cfd-limit)
+			  (c-got-face-at (point) c-literal-faces))))
 	       t)		      ; Continue the loop over pseudo matches.
 	      ((and c-opt-identifier-concat-key
 		    (match-string 1)
diff -r 9d58d1e3ab27 cc-langs.el
--- a/cc-langs.el	Sat Apr 27 17:07:23 2019 +0000
+++ b/cc-langs.el	Wed May 01 20:46:40 2019 +0000
@@ -964,6 +964,14 @@
 (c-lang-defvar c-opt-cpp-macro-define-id
   (c-lang-const c-opt-cpp-macro-define-id))
 
+(c-lang-defconst c-anchored-hash-define-no-parens
+  ;; Regexp matching everything up to the end of a cpp define which has no
+  ;; argument parentheses.  Or nil in languages which don't have them.
+  t (if (c-lang-const c-opt-cpp-macro-define)
+	(concat (c-lang-const c-anchored-cpp-prefix)
+		(c-lang-const c-opt-cpp-macro-define)
+		"[ \t]+\\(\\sw\\|_\\)+\\([^(a-zA-Z0-9_]\\|$\\)")))
+
 (c-lang-defconst c-cpp-expr-directives
   "List of cpp directives (without the prefix) that are followed by an
 expression."
@@ -1578,7 +1586,7 @@
   t (concat (c-lang-const c-comment-start-regexp)
 	    "\\|"
 	    (if (memq 'gen-string-delim c-emacs-features)
-		"\"|"
+		"\"\\|\\s|"
 	      "\"")))
 (c-lang-defvar c-literal-start-regexp (c-lang-const c-literal-start-regexp))
 
@@ -3152,24 +3160,40 @@
   ;; token that might precede such a construct, e.g. ';', '}' or '{'.
   ;; It's built from `c-decl-prefix-re'.
   ;;
-  ;; If the first submatch did not match, the match of the whole
-  ;; regexp is taken to be at the first token in the declaration.
-  ;; `c-decl-start-re' is not checked in this case.
+  ;; If the first submatch did not match, we have either a #define construct
+  ;; without parentheses or the match of the whole regexp is taken to be at
+  ;; the first token in the declaration.  `c-decl-start-re' is not checked in
+  ;; these cases.
   ;;
   ;; Design note: The reason the same regexp is used to match both
   ;; tokens that precede declarations and start them is to avoid an
   ;; extra regexp search from the previous declaration spot in
   ;; `c-find-decl-spots'.  Users of `c-find-decl-spots' also count on
-  ;; that it finds all declaration/cast/label starts in approximately
+  ;; it finding all declaration/cast/label starts in approximately
   ;; linear order, so we can't do the searches in two separate passes.
-  t (if (c-lang-const c-decl-start-kwds)
-	(concat (c-lang-const c-decl-prefix-re)
-		"\\|"
-		(c-make-keywords-re t (c-lang-const c-decl-start-kwds)))
-      (c-lang-const c-decl-prefix-re)))
+  t (cond
+     ((and (c-lang-const c-decl-start-kwds)
+	   (c-lang-const c-anchored-hash-define-no-parens))
+      (concat (c-lang-const c-decl-prefix-re)
+	      "\\|" (c-lang-const c-anchored-hash-define-no-parens)
+	      "\\|" (c-make-keywords-re t (c-lang-const c-decl-start-kwds))))
+     ((c-lang-const c-decl-start-kwds)
+      (concat (c-lang-const c-decl-prefix-re)
+	      "\\|" (c-make-keywords-re t (c-lang-const c-decl-start-kwds))))
+     ((c-lang-const c-anchored-hash-define-no-parens)
+      (concat (c-lang-const c-decl-prefix-re)
+	      "\\|" (c-lang-const c-anchored-hash-define-no-parens)))
+     (t (c-lang-const c-decl-prefix-re))))
 (c-lang-defvar c-decl-prefix-or-start-re
   (c-lang-const c-decl-prefix-or-start-re))
 
+(c-lang-defconst c-dposr-cpp-macro-depth
+  ;; The match number of `c-anchored-hash-define-no-parens''s first match
+  ;; within `c-decl-prefix-or-start-re', or nil if there is no such component.
+  t (if (c-lang-const c-anchored-hash-define-no-parens)
+	(1+ (regexp-opt-depth (c-lang-const c-decl-prefix-re)))))
+(c-lang-defvar c-dposr-cpp-macro-depth (c-lang-const c-dposr-cpp-macro-depth))
+
 (c-lang-defconst c-cast-parens
   ;; List containing the paren characters that can open a cast, or nil in
   ;; languages without casts.


> > Best regards,
> > Mauro.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#35454: 26.2.50; CC-Mode fontification fails inside macro

[Mauro Aranda]

[-- Attachment #1: Type: text/plain, Size: 1603 bytes --]

Alan Mackenzie <acm@muc.de> writes:

> Hello again, Mauro.

Hello Alan.  Thanks for looking into this bug!

> Please try out the patch below.  On my system, it corrects the
> fontification in both your test file and editfns.c.

I've applied the patch and tried the recipe I provided, and it works fine.

However, when I visit editfns.c and search for EXTRA_CONTEXT_FIELDS,
like I said in my report, I see the following problem with this variables:
struct buffer *buffer_a;
struct buffer *buffer_b;
unsigned char *deletions;
unsigned char *insertions;

All but deletions have face font-lock-variable-name-face.

I can't seem to come up with a simple recipe to reproduce the problem,
so I refer you to that part of editfns.c.

All the following steps, separately with emacs -Q (or you could kill the
buffer if you want)
1) C-x C-f editfns.c
C-s extra RET
I observe deletions without its correspondent face and if I type:
SPC DEL
deletions gets font-lock-variable-name-face face.  However, if I
revert the buffer with M-x revert-buffer RET yes RET buffer_a, deletions
and the first 'buffer' lose their faces.

2) C-x C-f editfns.c
C-s deletions RET
I see that deletions has the right face.  But
M-x revert-buffer
makes it lose it (but *buffer_a keeps its face).

3) C-x C-f editfns.c
C-s extra RET
deletions without font-lock-variable-name-face.
C-l C-l
M-x revert-buffer
deletions now has font-lock-variable-name-face.

That is all the testing I could do, sorry for not being able to come up
with a better recipe.  Let me know if you see the same behavior, or what
else I could try.

Best regards,
Mauro.

[-- Attachment #2: Type: text/html, Size: 1922 bytes --]

bug#35454: 26.2.50; CC-Mode fontification fails inside macro

[Alan Mackenzie]

Hello, Mauro.

On Wed, May 01, 2019 at 19:31:48 -0300, Mauro Aranda wrote:
> Alan Mackenzie <acm@muc.de> writes:

> Hello Alan.  Thanks for looking into this bug!

> > Please try out the patch below.  On my system, it corrects the
> > fontification in both your test file and editfns.c.

> I've applied the patch and tried the recipe I provided, and it works fine.

> However, when I visit editfns.c and search for EXTRA_CONTEXT_FIELDS,
> like I said in my report, I see the following problem with this variables:
> struct buffer *buffer_a;
> struct buffer *buffer_b;
> unsigned char *deletions;
> unsigned char *insertions;

> All but deletions have face font-lock-variable-name-face.

> I can't seem to come up with a simple recipe to reproduce the problem,
> so I refer you to that part of editfns.c.

Thanks, I didn't notice these last night, but I can see them now.

> All the following steps, separately with emacs -Q (or you could kill the
> buffer if you want)
> 1) C-x C-f editfns.c
> C-s extra RET
> I observe deletions without its correspondent face and if I type:
> SPC DEL
> deletions gets font-lock-variable-name-face face.  However, if I
> revert the buffer with M-x revert-buffer RET yes RET buffer_a, deletions
> and the first 'buffer' lose their faces.

The problem with "deletions" seems to be triggered by the 2-line comment
in the macro not having a backslash escaping the \n.  In nearly 30 years
hacking C, I've never seen this before, and didn't even know it was
valid syntax.  However, this means at least four very commonly used
functions (c-beginning-of-macro, c-end-of-macro, c-forward-sws, and
c-backward-sws) are going to have to be amended to deal with it, and
this is inevitably going to make CC Mode slower.  :-(

I can't see at the moment any pattern with the fontification on
buffer_a.  Sometimes just scrolling the buffer a few lines makes
buffer_a lose its fontification.  But if an empty pair of parentheses is
put after EXTRA_CONTEXT_FIELDS, all these problems go away.  So they
seem related to what I half-fixed last night.

> 2) C-x C-f editfns.c
> C-s deletions RET
> I see that deletions has the right face.  But
> M-x revert-buffer
> makes it lose it (but *buffer_a keeps its face).

> 3) C-x C-f editfns.c
> C-s extra RET
> deletions without font-lock-variable-name-face.
> C-l C-l
> M-x revert-buffer
> deletions now has font-lock-variable-name-face.

> That is all the testing I could do, sorry for not being able to come up
> with a better recipe.  Let me know if you see the same behavior, or what
> else I could try.

Well, that seems like quite a lot of testing to me.  :-)  Thanks for
doing it and reporting back to me so quickly.

I'll be working on the outstanding bugs here in the next few days.

> Best regards,
> Mauro.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#35454: 26.2.50; CC-Mode fontification fails inside macro

[Alan Mackenzie]

Hello again, Mauro.

On Thu, May 02, 2019 at 08:57:14 +0000, Alan Mackenzie wrote:
> On Wed, May 01, 2019 at 19:31:48 -0300, Mauro Aranda wrote:
> > Alan Mackenzie <acm@muc.de> writes:

> > I've applied the patch and tried the recipe I provided, and it works fine.

> > However, when I visit editfns.c and search for EXTRA_CONTEXT_FIELDS,
> > like I said in my report, I see the following problem with this variables:
> > struct buffer *buffer_a;
> > struct buffer *buffer_b;
> > unsigned char *deletions;
> > unsigned char *insertions;

> > All but deletions have face font-lock-variable-name-face.

[ .... ]

> The problem with "deletions" seems to be triggered by the 2-line comment
> in the macro not having a backslash escaping the \n.  In nearly 30 years
> hacking C, I've never seen this before, and didn't even know it was
> valid syntax.  However, this means at least four very commonly used
> functions (c-beginning-of-macro, c-end-of-macro, c-forward-sws, and
> c-backward-sws) are going to have to be amended to deal with it, and
> this is inevitably going to make CC Mode slower.  :-(

I've just committed a fix to multiline comments in macros not having
escaped newlines.  This seems to solve the problem with the variable
"deletions".  That should be half the battle won.  As usual, please feel
free to test it for me.

In the end, it didn't make CC Mode more than around 0.5% slower.

I haven't yet tried combining yesterday's patch with the fix I've just
committed, but if we're lucky, the two together might solve the entire
bug.

[ .... ]

> > Best regards,
> > Mauro.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#35454: 26.2.50; CC-Mode fontification fails inside macro

[Alan Mackenzie]

Hello, Mauro.

On Wed, May 01, 2019 at 19:31:48 -0300, Mauro Aranda wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > Hello again, Mauro.

> Hello Alan.  Thanks for looking into this bug!

> > Please try out the patch below.  On my system, it corrects the
> > fontification in both your test file and editfns.c.

> I've applied the patch and tried the recipe I provided, and it works fine.

> However, when I visit editfns.c and search for EXTRA_CONTEXT_FIELDS,
> like I said in my report, I see the following problem with this variables:
> struct buffer *buffer_a;
> struct buffer *buffer_b;
> unsigned char *deletions;
> unsigned char *insertions;

> All but deletions have face font-lock-variable-name-face.

> I can't seem to come up with a simple recipe to reproduce the problem,
> so I refer you to that part of editfns.c.

> All the following steps, separately with emacs -Q (or you could kill the
> buffer if you want)
> 1) C-x C-f editfns.c
> C-s extra RET
> I observe deletions without its correspondent face and if I type:
> SPC DEL
> deletions gets font-lock-variable-name-face face.  However, if I
> revert the buffer with M-x revert-buffer RET yes RET buffer_a, deletions
> and the first 'buffer' lose their faces.

> 2) C-x C-f editfns.c
> C-s deletions RET
> I see that deletions has the right face.  But
> M-x revert-buffer
> makes it lose it (but *buffer_a keeps its face).

> 3) C-x C-f editfns.c
> C-s extra RET
> deletions without font-lock-variable-name-face.
> C-l C-l
> M-x revert-buffer
> deletions now has font-lock-variable-name-face.

> That is all the testing I could do, sorry for not being able to come up
> with a better recipe.  Let me know if you see the same behavior, or what
> else I could try.

I believe I have now fixed all these bugs, and indeed I've committed the
fix to the master branch.

Would you please be so good as to give this fix a "final" test, and if
everything is OK, then I can close the bug.

Thanks!

> Best regards,
> Mauro.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#35454: 26.2.50; CC-Mode fontification fails inside macro

[Mauro Aranda]

[-- Attachment #1: Type: text/plain, Size: 402 bytes --]

Alan Mackenzie <acm@muc.de> writes:

> Hello, Mauro.
> I believe I have now fixed all these bugs, and indeed I've committed the
> fix to the master branch.
>
> Would you please be so good as to give this fix a "final" test, and if
> everything is OK, then I can close the bug.

Hello Alan.

I tried the fix, and it works great!  All the problems I noticed are solved.

Thank you.

Best regards,
Mauro.

[-- Attachment #2: Type: text/html, Size: 568 bytes --]

bug#35454: 26.2.50; CC-Mode fontification fails inside macro

[Alan Mackenzie]

Hello, Mauro.

On Thu, May 02, 2019 at 18:44:02 -0300, Mauro Aranda wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > I believe I have now fixed all these bugs, and indeed I've committed the
> > fix to the master branch.

> > Would you please be so good as to give this fix a "final" test, and if
> > everything is OK, then I can close the bug.

> Hello Alan.

> I tried the fix, and it works great!  All the problems I noticed are solved.

> Thank you.

That's great.  I'm closing the bug.  Thanks!

> Best regards,
> Mauro.

-- 
Alan Mackenzie (Nuremberg, Germany).