Re: Template types not correctly highlighted in C++

[Lud <lu...@lu...>]

Hello Allan,

Thank you for your patch.

 > 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

On 04/05/2017 07:52 PM, Alan Mackenzie wrote:
> Hello, Ludwig.
> 
> On Tue, Mar 28, 2017 at 08:14:29 +0100, Lud wrote:
>> Alan,
> 
> 
>>   > 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.
> 
>>   > 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.
> 
> 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
> 
> 
> 
>> Ludwig
>