unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Alan Mackenzie <acm@muc.de>
To: Naofumi Yasufuku <naofumi@yasufuku.dev>
Cc: 57318@debbugs.gnu.org
Subject: bug#57318: 29.0.50; c++-mode: Functions are shown as variables: partial template specialization
Date: Tue, 30 Aug 2022 20:00:00 +0000	[thread overview]
Message-ID: <Yw5sQJcT9ypvMX14@ACM> (raw)
In-Reply-To: <m1v8qdx78r.fsf@yasufuku.dev>

Hello, Naofumi.

Thank you indeed for your bug report, and thanks even more for
diagnosing the cause and writing a patch!

I'm truly astounded that you managed to debug this problem, hidden as it
was in the fontification parts of CC Mode, which are difficult to run
with the debugger.  Respect!

Looking at your patch, I can see one or two problems with it.  The main
one is that after encountering a template opener "<", I'm a bit worried
about the accuracy of checking for the matching ">".

Looking further into the problem, it seems the misfontification bug was
triggered by the commas in the template expressions.  When there's a
comma there, the (c-forward-<>-arglist ...) call (which "; Should always
work") fails.  This failure is caused by the dynamic binding of
c-restricted-<>-arglists to t.

When this dynamic binding is instead at nil (rather than t), the
fontification works properly, with functions fontified as functions.
The reason for this faulty binding was fixing a bug report where
scrolling a buffer containing lots of templates was very slow.  Binding
c-restricted-<>-arglists to t "solved" that slowness.

That slowness had been caused by inefficient scanning of the template
structures.  I've now identified and implemented an alternative
optimisation, which doesn't cause problems with commas in templates.

So I must apologize that I won't be using your patch, but I thoroughly
appreciate the time you must have spent debugging the problem.

Can I ask you, please, to apply and test my proposed patch, which I
enclose below.  Would you please test it on your real C++ source code,
and either confirm it really works, or say what's still faulty about it.
If there are any problems with applying the patch, feel free to send me
personal email.



diff -r 8f28cf5aae11 cc-engine.el
--- a/cc-engine.el	Sat Aug 27 09:10:03 2022 +0000
+++ b/cc-engine.el	Tue Aug 30 19:46:56 2022 +0000
@@ -146,6 +146,11 @@
 ;;       Put on the brace which introduces a brace list and on the commas
 ;;       which separate the elements within it.
 ;;
+;; 'c-<>-c-types-set
+;;   This property is set on an opening angle bracket, and indicates that
+;;   any "," separators within the template/generic expression have been
+;;   marked with a 'c-type property value 'c-<>-arg-sep (see above).
+;;
 ;; 'c-awk-NL-prop
 ;;   Used in AWK mode to mark the various kinds of newlines.  See
 ;;   cc-awk.el.
@@ -6153,7 +6158,7 @@
 			(forward-char))))
 	  (backward-char)
 	  (if (let ((c-parse-and-markup-<>-arglists t)
-		    (c-restricted-<>-arglists t))
+		    c-restricted-<>-arglists)
 		(c-forward-<>-arglist nil)) ; Should always work.
 	      (when (> (point) to)
 		(setq bound-<> (point)))
@@ -8526,9 +8531,9 @@
 	arg-start-pos)
     ;; If the '<' has paren open syntax then we've marked it as an angle
     ;; bracket arglist before, so skip to the end.
-    (if (and (not c-parse-and-markup-<>-arglists)
-	     syntax-table-prop-on-<)
-
+    (if (and syntax-table-prop-on-<
+	     (or (not c-parse-and-markup-<>-arglists)
+		 (c-get-char-property (point) 'c-<>-c-types-set)))
 	(progn
 	  (forward-char)
 	  (if (and (c-go-up-list-forward)
@@ -8623,6 +8628,7 @@
 			       (c-unmark-<->-as-paren (point)))))
 		      (c-mark-<-as-paren start)
 		      (c-mark->-as-paren (1- (point)))
+		      (c-put-char-property start 'c-<>-c-types-set t)
 		      (c-truncate-lit-pos-cache start))
 		    (setq res t)
 		    nil))		; Exit the loop.
diff -r 8f28cf5aae11 cc-fonts.el
--- a/cc-fonts.el	Sat Aug 27 09:10:03 2022 +0000
+++ b/cc-fonts.el	Tue Aug 30 19:46:56 2022 +0000
@@ -937,17 +937,16 @@
     (save-excursion
       (let ((pos (point)))
 	(c-backward-syntactic-ws (max (- (point) 500) (point-min)))
-	(c-clear-char-properties
-	 (if (and (not (bobp))
-		  (memq (c-get-char-property (1- (point)) 'c-type)
-			'(c-decl-arg-start
-			  c-decl-end
-			  c-decl-id-start
-			  c-decl-type-start
-			  c-not-decl)))
-	     (1- (point))
-	   pos)
-	 limit 'c-type)))
+	(when (and (not (bobp))
+		   (memq (c-get-char-property (1- (point)) 'c-type)
+			 '(c-decl-arg-start
+			   c-decl-end
+			   c-decl-id-start
+			   c-decl-type-start
+			   c-not-decl)))
+	  (setq pos (1- (point))))
+	(c-clear-char-properties pos limit 'c-type)
+	(c-clear-char-properties pos limit 'c-<>-c-types-set)))
 
     ;; Update `c-state-cache' to the beginning of the region.  This will
     ;; make `c-beginning-of-syntax' go faster when it's used later on,



On Sun, Aug 28, 2022 at 14:33:08 +0900, Naofumi Yasufuku wrote:
> Naofumi Yasufuku <naofumi@yasufuku.dev> writes:

> > Hello,
> >
> > In the case of C++ partial template specialization [1], functions are
> > shown as variables.  Please look at the attached screenshots.
> >
> >   1. emacs -Q
> >   2. open attached C++ code "specialization.cc"
> >
> >   attachments:
> >   - NG_cc-partial-specialization.png
> >   - OK_cc-partial-specialization.png
> >   - specialization.cc
> >
> [..snip..]
> >
> > After a brief investigation, the attached cc-engine.el patch seems to
> > solve this issue at least, but it is not tested enough.
> >
> >   attachments:
> >   - 0001-cc-mode-Fix-function-fontification-in-the-case-of-C-.patch
> >

> Previous patch still has a problem with sizeof() specializations.
> Updated patch and screenshots are attached.

>   attachments:
>   - 0001-cc-mode-Fix-function-fontification-in-the-case-of-C-v2.patch
>   - NG_cc-partial-specialization-v1.png
>   - OK_cc-partial-specialization-v2.png
>   - specialization.cc

> -------------------------------------------------------------
> template<typename T1, size_t>
> class TwoSz {
> public:
>     TwoSz() {}
> private:
>     T1 _t1;
> };

> template<typename T>
> class TwoSz<T, sizeof(int)> {
> public:
>     TwoSz() {}
> private:
>     T _t1;
> };
> -------------------------------------------------------------
> $ ./specialization
> One<T>: primary
> One<char>: specialization
> Two<T1, T2>: primary
> Two<T, char>: specialization
> Two<T, T*>: specialization
> Two<T, T&>: specialization
> Two<T, T[]>: specialization
> Two<char, char*>: specialization
> Two<char, char&>: specialization
> Two<char, char[]>: specialization
> TwoSz<T1, size_t>: primary
> TwoSz<T, sizeof(int)>: specialization
> $
> -------------------------------------------------------------

> Best regards,
>   Naofumi

-- 
Alan Mackenzie (Nuremberg, Germany).





  reply	other threads:[~2022-08-30 20:00 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-21 13:27 bug#57318: 29.0.50; c++-mode: Functions are shown as variables: partial template specialization Naofumi Yasufuku
2022-08-28  5:33 ` Naofumi Yasufuku
2022-08-30 20:00   ` Alan Mackenzie [this message]
2022-08-31 16:01     ` Naofumi Yasufuku
2022-08-31 19:26       ` Alan Mackenzie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Yw5sQJcT9ypvMX14@ACM \
    --to=acm@muc.de \
    --cc=57318@debbugs.gnu.org \
    --cc=naofumi@yasufuku.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).