Template types not correctly highlighted in C++

[l***@lud.cc]

Hello Alan,

if you consider the below C++ code:

// NotHighlighted is defined in another file.
template<typename Baz>
class Foo : public Bar<Baz> {
std::vector<NotHighlighted> foo;
}

With `emacs -Q` I get the display as shown in the attached file
template0.png. Note, the symbol "NotHighlighted" is not highlighted like
the symbol "Baz".

If I erase '>' and put it back (just after "NotHighlighted"), the font
lock seems to be refreshed and this time the symbol "NotHighlighted" is
highlighted (see template1.png).

Note, if I add the following C++ line "class NotHighlighted;", it is
correct (cf. template2.png).

Can you tell me if is an expected behavior? I would expected the
template argument to be colored in an homogeneous way across the code.
template[12].png seem correct.

Thanks for your time
Ludwig

version: GNU Emacs 26.0.50.1 (x86_64-unknown-linux-gnu, GTK+ Version
2.24.28) of 2017-02-13

[Alan Mackenzie]

Hello, Ludwig.

Thanks for the bug report, and thanks even more for cutting it down to a
nice, small, easy to work with snippet.

Post by l***@lud.cc
Hello Alan,
// NotHighlighted is defined in another file.
template<typename Baz>
class Foo : public Bar<Baz> {
std::vector<NotHighlighted> foo;
}
With `emacs -Q` I get the display as shown in the attached file
template0.png. Note, the symbol "NotHighlighted" is not highlighted like
the symbol "Baz".

This is actually correct. CC Mode doesn't know whether or not
"NotHighlighted" is a type or not; it could easily be a constant
integer. (Yes, I know that std::vector always takes a type, but this
goes into semantics more than CC Mode wants to).

Post by l***@lud.cc
If I erase '>' and put it back (just after "NotHighlighted"), the font
lock seems to be refreshed and this time the symbol "NotHighlighted" is
highlighted (see template1.png).

Yes, this seems to be a bug. ;-) That sequence of operations shouldn't
cause the fontification of "NotHighlighted". I'm trying to track this
down at the moment.

CC Mode maintains a collection of "known types" for each buffer -
identifiers which are known from their contexts to be types. Somehow,
"NotHighlighted" has got into this collection. I will figure out soon
how and why.

Post by l***@lud.cc
Note, if I add the following C++ line "class NotHighlighted;", it is
correct (cf. template2.png).

When you do this, "NotHighlighted" becomes a "known type" and the
fontification is then correct.

Post by l***@lud.cc
Can you tell me if is an expected behavior? I would expected the
template argument to be colored in an homogeneous way across the code.
template[12].png seem correct.

See above.

The more fundamental question is whether an ambiguous template argument
should be fontified as a type or not. Doing so will inevitably lead to
errors in places where the identifier actually isn't a type.

Post by l***@lud.cc
Thanks for your time

Thank you again for the report! I'll be back in touch again when I've
made more progress.

Post by l***@lud.cc
Ludwig
version: GNU Emacs 26.0.50.1 (x86_64-unknown-linux-gnu, GTK+ Version
2.24.28) of 2017-02-13

[Lud]

Alan,

Post by Alan Mackenzie
Yes, this seems to be a bug. ;-) That sequence of operations shouldn't
cause the fontification of "NotHighlighted". I'm trying to track this
down at the moment.
CC Mode maintains a collection of "known types" for each buffer -
identifiers which are known from their contexts to be types. Somehow,
"NotHighlighted" has got into this collection. I will figure out soon
how and why.

Let me know when I can help for testing.

Post by Alan Mackenzie
The more fundamental question is whether an ambiguous template argument
should be fontified as a type or not. Doing so will inevitably lead to
errors in places where the identifier actually isn't a type.

I think it is better to not font lock ambiguous template arguments,
because of the following reasons:

1. correctness - as you said, "errors [...] where the identifier
actually isn't a type"
2. less font lock is a speedup to render the source file because it is a
heavy process which can be annoying for large files.

Ludwig

Post by Alan Mackenzie
Hello, Ludwig.
Thanks for the bug report, and thanks even more for cutting it down to a
nice, small, easy to work with snippet.

Post by l***@lud.cc
Hello Alan,
// NotHighlighted is defined in another file.
template<typename Baz>
class Foo : public Bar<Baz> {
std::vector<NotHighlighted> foo;
}
With `emacs -Q` I get the display as shown in the attached file
template0.png. Note, the symbol "NotHighlighted" is not highlighted like
the symbol "Baz".

This is actually correct. CC Mode doesn't know whether or not
"NotHighlighted" is a type or not; it could easily be a constant
integer. (Yes, I know that std::vector always takes a type, but this
goes into semantics more than CC Mode wants to).

Post by l***@lud.cc
If I erase '>' and put it back (just after "NotHighlighted"), the font
lock seems to be refreshed and this time the symbol "NotHighlighted" is
highlighted (see template1.png).

Yes, this seems to be a bug. ;-) That sequence of operations shouldn't
cause the fontification of "NotHighlighted". I'm trying to track this
down at the moment.
CC Mode maintains a collection of "known types" for each buffer -
identifiers which are known from their contexts to be types. Somehow,
"NotHighlighted" has got into this collection. I will figure out soon
how and why.

Post by l***@lud.cc
Note, if I add the following C++ line "class NotHighlighted;", it is
correct (cf. template2.png).

When you do this, "NotHighlighted" becomes a "known type" and the
fontification is then correct.

Post by l***@lud.cc
Can you tell me if is an expected behavior? I would expected the
template argument to be colored in an homogeneous way across the code.
template[12].png seem correct.

See above.
The more fundamental question is whether an ambiguous template argument
should be fontified as a type or not. Doing so will inevitably lead to
errors in places where the identifier actually isn't a type.

Post by l***@lud.cc
Thanks for your time

Thank you again for the report! I'll be back in touch again when I've
made more progress.

Post by l***@lud.cc
Ludwig
version: GNU Emacs 26.0.50.1 (x86_64-unknown-linux-gnu, GTK+ Version
2.24.28) of 2017-02-13

[Alan Mackenzie]

Hello, Ludwig.

Post by Lud
Alan,

Post by Alan Mackenzie
Yes, this seems to be a bug. ;-) That sequence of operations shouldn't
cause the fontification of "NotHighlighted". I'm trying to track this
down at the moment.
CC Mode maintains a collection of "known types" for each buffer -
identifiers which are known from their contexts to be types. Somehow,
"NotHighlighted" has got into this collection. I will figure out soon
how and why.

Let me know when I can help for testing.

Please apply the patch below. I think it works on the test code in your
original bug report, but would you please try it out on your normal code,
and either let me know that it works, or tell me what still needs fixing.

This patch attempts to remove the "ambiguity" of font locking which
happened with deleting and reinserting the ">", and such like.

Post by Lud

Post by Alan Mackenzie
The more fundamental question is whether an ambiguous template argument
should be fontified as a type or not. Doing so will inevitably lead to
errors in places where the identifier actually isn't a type.

I think it is better to not font lock ambiguous template arguments,
1. correctness - as you said, "errors [...] where the identifier
actually isn't a type"
2. less font lock is a speedup to render the source file because it is a
heavy process which can be annoying for large files.

I agree with you there, I think, except it won't really help the speedup,
much.

Anyway, here is the patch. (Feel free to send me private mail if you
want any help applying it). After applying it, just byte-compile
cc-engine.el and cc-mode.el the usual way, and restart Emacs. (Or
however you usually do it. ;-)



diff -r 79b94d28dad4 cc-engine.el
--- a/cc-engine.el Sun Mar 19 16:33:03 2017 +0000
+++ b/cc-engine.el Tue Apr 04 20:32:35 2017 +0000
@@ -6005,6 +6005,13 @@
;; Clears `c-found-types'.
(setq c-found-types (make-vector 53 0)))

+(defun c-copy-found-types ()
+ (let ((copy (make-vector 53 0)))
+ (mapatoms (lambda (sym)
+ (intern (symbol-name sym) copy))
+ c-found-types)
+ copy))
+
(defun c-add-type (from to)
;; Add the given region as a type in `c-found-types'. If the region
;; doesn't match an existing type but there is a type which is equal
@@ -6965,6 +6972,7 @@
;; This function might do hidden buffer changes.

(let ((start (point))
+ (old-found-types (c-copy-found-types))
;; If `c-record-type-identifiers' is set then activate
;; recording of any found types that constitute an argument in
;; the arglist.
@@ -6980,6 +6988,7 @@
(nconc c-record-found-types c-record-type-identifiers)))
t)

+ (setq c-found-types old-found-types)
(goto-char start)
nil)))

diff -r 79b94d28dad4 cc-mode.el
--- a/cc-mode.el Sun Mar 19 16:33:03 2017 +0000
+++ b/cc-mode.el Tue Apr 04 20:32:35 2017 +0000
@@ -429,27 +429,36 @@
t))))

(defun c-unfind-coalesced-tokens (beg end)
- ;; unless the non-empty region (beg end) is entirely WS and there's at
- ;; least one character of WS just before or after this region, remove
- ;; the tokens which touch the region from `c-found-types' should they
- ;; be present.
- (or (c-partial-ws-p beg end)
- (save-excursion
- (progn
- (goto-char beg)
- (or (eq beg (point-min))
- (c-skip-ws-backward (1- beg))
- (/= (point) beg)
- (= (c-backward-token-2) 1)
- (c-unfind-type (buffer-substring-no-properties
- (point) beg)))
- (goto-char end)
- (or (eq end (point-max))
- (c-skip-ws-forward (1+ end))
- (/= (point) end)
- (progn (forward-char) (c-end-of-current-token) nil)
- (c-unfind-type (buffer-substring-no-properties
- end (point))))))))
+ ;; If removing the region (beg end) would coalesce an identifier ending at
+ ;; beg with an identifier (fragment) beginning at end, or an identifier
+ ;; fragment ending at beg with an identifier beginning at end, remove the
+ ;; pertinent identifier(s) from `c-found-types'.
+ (save-excursion
+ (when (< beg end)
+ (goto-char beg)
+ (when
+ (and (not (bobp))
+ (progn (c-backward-syntactic-ws) (eq (point) beg))
+ (/= (skip-chars-backward c-symbol-chars (1- (point))) 0)
+ (progn (goto-char beg) (c-forward-syntactic-ws) (<= (point) end))
+ (> (point) beg)
+ (goto-char end)
+ (looking-at c-symbol-char-key))
+ (goto-char beg)
+ (c-simple-skip-symbol-backward)
+ (c-unfind-type (buffer-substring-no-properties (point) beg)))
+
+ (goto-char end)
+ (when
+ (and (not (eobp))
+ (progn (c-forward-syntactic-ws) (eq (point) end))
+ (looking-at c-symbol-char-key)
+ (progn (c-backward-syntactic-ws) (>= (point) beg))
+ (< (point) end)
+ (/= (skip-chars-backward c-symbol-chars (1- (point))) 0))
+ (goto-char (1+ end))
+ (c-end-of-current-token)
+ (c-unfind-type (buffer-substring-no-properties end (point)))))))

;; c-maybe-stale-found-type records a place near the region being
;; changed where an element of `found-types' might become stale. It

Post by Lud
Ludwig

[Lud]

Hello Allan,

Thank you for your patch.

Post by Alan Mackenzie
Please apply the patch below. I think it works on the test code in your
original bug report, but would you please try it out on your normal code,
and either let me know that it works, or tell me what still needs fixing.
This patch attempts to remove the "ambiguity" of font locking which
happened with deleting and reinserting the ">", and such like.

On my code, the result is better, but I still have some not consistent
behaviors. I tested with the below snippet of code, and the master
branch the git repo of emacs:

#include <vector>

struct NotHighlighted{ int a; };

namespace abc { struct NotHighlighted2{ int b; }; }

// $ g++ test.cpp # should compile correctly
int main () {
std::vector<NotHighlighted> foo;
std::vector<abc::NotHighlighted2> bar;
}

After my tests, here are my results:

1. Open the file with the snippet, types in both vector are not
higlighted. I expected to be higlighted?

2. Remove and put back '>' of 'std::vector<NotHighlighted> foo' makes
the higlight on for the type.

3. Add a '2' at the end of the type 'std::vector<NotHighlighted> foo',
highlight is still on. I expected to be off?

4. Replace the '2' by a '3' for 'std::vector<NotHighlighted> foo',
highlight of the type is off. Looks good to me.

5. Remove and put back '>' of 'std::vector<abc::NotHighlighted> bar',
the highlight of the type is still off. I expect to be on with namespaces?

Let me know your thoughts about these points.

King regards
Ludwig

Post by Alan Mackenzie
Hello, Ludwig.

Post by Lud
Alan,

Post by Alan Mackenzie
Yes, this seems to be a bug. ;-) That sequence of operations shouldn't
cause the fontification of "NotHighlighted". I'm trying to track this
down at the moment.
CC Mode maintains a collection of "known types" for each buffer -
identifiers which are known from their contexts to be types. Somehow,
"NotHighlighted" has got into this collection. I will figure out soon
how and why.

Let me know when I can help for testing.

Please apply the patch below. I think it works on the test code in your
original bug report, but would you please try it out on your normal code,
and either let me know that it works, or tell me what still needs fixing.
This patch attempts to remove the "ambiguity" of font locking which
happened with deleting and reinserting the ">", and such like.

Post by Lud

Post by Alan Mackenzie
The more fundamental question is whether an ambiguous template argument
should be fontified as a type or not. Doing so will inevitably lead to
errors in places where the identifier actually isn't a type.

I think it is better to not font lock ambiguous template arguments,
1. correctness - as you said, "errors [...] where the identifier
actually isn't a type"
2. less font lock is a speedup to render the source file because it is a
heavy process which can be annoying for large files.

I agree with you there, I think, except it won't really help the speedup,
much.
Anyway, here is the patch. (Feel free to send me private mail if you
want any help applying it). After applying it, just byte-compile
cc-engine.el and cc-mode.el the usual way, and restart Emacs. (Or
however you usually do it. ;-)
diff -r 79b94d28dad4 cc-engine.el
--- a/cc-engine.el Sun Mar 19 16:33:03 2017 +0000
+++ b/cc-engine.el Tue Apr 04 20:32:35 2017 +0000
@@ -6005,6 +6005,13 @@
;; Clears `c-found-types'.
(setq c-found-types (make-vector 53 0)))
+(defun c-copy-found-types ()
+ (let ((copy (make-vector 53 0)))
+ (mapatoms (lambda (sym)
+ (intern (symbol-name sym) copy))
+ c-found-types)
+ copy))
+
(defun c-add-type (from to)
;; Add the given region as a type in `c-found-types'. If the region
;; doesn't match an existing type but there is a type which is equal
@@ -6965,6 +6972,7 @@
;; This function might do hidden buffer changes.
(let ((start (point))
+ (old-found-types (c-copy-found-types))
;; If `c-record-type-identifiers' is set then activate
;; recording of any found types that constitute an argument in
;; the arglist.
@@ -6980,6 +6988,7 @@
(nconc c-record-found-types c-record-type-identifiers)))
t)
+ (setq c-found-types old-found-types)
(goto-char start)
nil)))
diff -r 79b94d28dad4 cc-mode.el
--- a/cc-mode.el Sun Mar 19 16:33:03 2017 +0000
+++ b/cc-mode.el Tue Apr 04 20:32:35 2017 +0000
@@ -429,27 +429,36 @@
t))))
(defun c-unfind-coalesced-tokens (beg end)
- ;; unless the non-empty region (beg end) is entirely WS and there's at
- ;; least one character of WS just before or after this region, remove
- ;; the tokens which touch the region from `c-found-types' should they
- ;; be present.
- (or (c-partial-ws-p beg end)
- (save-excursion
- (progn
- (goto-char beg)
- (or (eq beg (point-min))
- (c-skip-ws-backward (1- beg))
- (/= (point) beg)
- (= (c-backward-token-2) 1)
- (c-unfind-type (buffer-substring-no-properties
- (point) beg)))
- (goto-char end)
- (or (eq end (point-max))
- (c-skip-ws-forward (1+ end))
- (/= (point) end)
- (progn (forward-char) (c-end-of-current-token) nil)
- (c-unfind-type (buffer-substring-no-properties
- end (point))))))))
+ ;; If removing the region (beg end) would coalesce an identifier ending at
+ ;; beg with an identifier (fragment) beginning at end, or an identifier
+ ;; fragment ending at beg with an identifier beginning at end, remove the
+ ;; pertinent identifier(s) from `c-found-types'.
+ (save-excursion
+ (when (< beg end)
+ (goto-char beg)
+ (when
+ (and (not (bobp))
+ (progn (c-backward-syntactic-ws) (eq (point) beg))
+ (/= (skip-chars-backward c-symbol-chars (1- (point))) 0)
+ (progn (goto-char beg) (c-forward-syntactic-ws) (<= (point) end))
+ (> (point) beg)
+ (goto-char end)
+ (looking-at c-symbol-char-key))
+ (goto-char beg)
+ (c-simple-skip-symbol-backward)
+ (c-unfind-type (buffer-substring-no-properties (point) beg)))
+
+ (goto-char end)
+ (when
+ (and (not (eobp))
+ (progn (c-forward-syntactic-ws) (eq (point) end))
+ (looking-at c-symbol-char-key)
+ (progn (c-backward-syntactic-ws) (>= (point) beg))
+ (< (point) end)
+ (/= (skip-chars-backward c-symbol-chars (1- (point))) 0))
+ (goto-char (1+ end))
+ (c-end-of-current-token)
+ (c-unfind-type (buffer-substring-no-properties end (point)))))))
;; c-maybe-stale-found-type records a place near the region being
;; changed where an element of `found-types' might become stale. It

Post by Lud
Ludwig

[Alan Mackenzie]

Hello, Ludwig.

Post by Lud
Hello Allan,
Thank you for your patch.

Post by Alan Mackenzie
Please apply the patch below. I think it works on the test code in your
original bug report, but would you please try it out on your normal code,
and either let me know that it works, or tell me what still needs fixing.
This patch attempts to remove the "ambiguity" of font locking which
happened with deleting and reinserting the ">", and such like.

On my code, the result is better, but I still have some not consistent
behaviors. I tested with the below snippet of code, and the master
1 #include <vector>
3 struct NotHighlighted{ int a; };
5 namespace abc { struct NotHighlighted2{ int b; }; }
7 // $ g++ test.cpp # should compile correctly
8 int main () {
9 std::vector<NotHighlighted> foo;
10 std::vector<abc::NotHighlighted2> bar;
11 }
1. Open the file with the snippet, types in both vector are not
higlighted. I expected to be higlighted?

What is happening with "NotHighlighted" is this: the identifier is
scanned on line 3 and entered into CC Mode's "found types" list. (This
records identifiers whose context shows they must be types). Then
"NotHighlighted2" is scanned on L5, and entered into the list. But in
doing so, "NotHighlighted" gets removed, as it consists of all but the
last character of the new identifier. The thinking here is that a new
identifier is going to be typed in one character at a time, and that the
list should not be clogged up by all the "partial identifiers".

You can verify this by replacing "NotHighlighted2" by "NotHighlighted22".
This identifier then gets fontified at mode start.

I'm not sure how to solve this.

I'm still working on the other problems. I'll get back to you later.

Post by Lud
2. Remove and put back '>' of 'std::vector<NotHighlighted> foo' makes
the higlight on for the type.
3. Add a '2' at the end of the type 'std::vector<NotHighlighted> foo',
highlight is still on. I expected to be off?
4. Replace the '2' by a '3' for 'std::vector<NotHighlighted> foo',
highlight of the type is off. Looks good to me.
5. Remove and put back '>' of 'std::vector<abc::NotHighlighted> bar',
the highlight of the type is still off. I expect to be on with namespaces?
Let me know your thoughts about these points.
King regards
Ludwig

[Alan Mackenzie]

Hello, Ludwig.

Sorry about the long delay. I got bogged down in debugging your other
bugs back in April, and haven't yet got around to solving them. So I
decided just to commit the patch which fixes your original bug. I _will_
get back to looking at the other bugs sometime.

The committed change is in both the CC Mode repository head, and the
Emacs master head.

Thanks, once more, for bringing these bugs to my attention, and doing so
in such a concise, easy to work with way.
--
Alan Mackenzie (Nuremberg, Germany).

Post by Lud
Hello Allan,
Thank you for your patch.

Post by Alan Mackenzie
Please apply the patch below. I think it works on the test code in your
original bug report, but would you please try it out on your normal code,
and either let me know that it works, or tell me what still needs fixing.
This patch attempts to remove the "ambiguity" of font locking which
happened with deleting and reinserting the ">", and such like.

On my code, the result is better, but I still have some not consistent
behaviors. I tested with the below snippet of code, and the master
#include <vector>
struct NotHighlighted{ int a; };
namespace abc { struct NotHighlighted2{ int b; }; }
// $ g++ test.cpp # should compile correctly
int main () {
std::vector<NotHighlighted> foo;
std::vector<abc::NotHighlighted2> bar;
}
1. Open the file with the snippet, types in both vector are not
higlighted. I expected to be higlighted?
2. Remove and put back '>' of 'std::vector<NotHighlighted> foo' makes
the higlight on for the type.
3. Add a '2' at the end of the type 'std::vector<NotHighlighted> foo',
highlight is still on. I expected to be off?
4. Replace the '2' by a '3' for 'std::vector<NotHighlighted> foo',
highlight of the type is off. Looks good to me.
5. Remove and put back '>' of 'std::vector<abc::NotHighlighted> bar',
the highlight of the type is still off. I expect to be on with namespaces?
Let me know your thoughts about these points.
King regards
Ludwig

Post by Alan Mackenzie
Hello, Ludwig.

Post by Lud
Alan,

Post by Alan Mackenzie
Yes, this seems to be a bug. ;-) That sequence of operations shouldn't
cause the fontification of "NotHighlighted". I'm trying to track this
down at the moment.
CC Mode maintains a collection of "known types" for each buffer -
identifiers which are known from their contexts to be types. Somehow,
"NotHighlighted" has got into this collection. I will figure out soon
how and why.

Let me know when I can help for testing.

Please apply the patch below. I think it works on the test code in your
original bug report, but would you please try it out on your normal code,
and either let me know that it works, or tell me what still needs fixing.
This patch attempts to remove the "ambiguity" of font locking which
happened with deleting and reinserting the ">", and such like.

Post by Lud

Post by Alan Mackenzie
The more fundamental question is whether an ambiguous template argument
should be fontified as a type or not. Doing so will inevitably lead to
errors in places where the identifier actually isn't a type.

I think it is better to not font lock ambiguous template arguments,
1. correctness - as you said, "errors [...] where the identifier
actually isn't a type"
2. less font lock is a speedup to render the source file because it is a
heavy process which can be annoying for large files.

I agree with you there, I think, except it won't really help the speedup,
much.
Anyway, here is the patch. (Feel free to send me private mail if you
want any help applying it). After applying it, just byte-compile
cc-engine.el and cc-mode.el the usual way, and restart Emacs. (Or
however you usually do it. ;-)

[ patch snipped ]

Post by Lud

Post by Alan Mackenzie

Post by Lud
Ludwig

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[Lud]

Hello Alan,

Thank you for your time as well. C++ is a complexe language. Let me know
when you'll need me to do some tests.

Ludwig

Post by Alan Mackenzie
Hello, Ludwig.
Sorry about the long delay. I got bogged down in debugging your other
bugs back in April, and haven't yet got around to solving them. So I
decided just to commit the patch which fixes your original bug. I _will_
get back to looking at the other bugs sometime.
The committed change is in both the CC Mode repository head, and the
Emacs master head.
Thanks, once more, for bringing these bugs to my attention, and doing so
in such a concise, easy to work with way.