unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#15415: 24.3.50; c++-mode fontification for constructors is inconsistent
@ 2013-09-19  2:44 Ivan Andrus
  2013-09-30  3:31 ` Ivan Andrus
  0 siblings, 1 reply; 5+ messages in thread
From: Ivan Andrus @ 2013-09-19  2:44 UTC (permalink / raw)
  To: 15415

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

Font locking of C++ constructors is somewhat inconsistent.  This is no
doubt complicated by the fact that unlike other function declarations
they "don't have a return type".

When a single argument is not used but named, the constructor is not
fontified (normally it's fontified with `font-lock-function-name-face').
If the keyword explicit is used, then the argument type is fontified as
a variable, and the constructor name is fontified as a type.  Perhaps
interestingly, naming the parameter or adding another parameter causes
fontification to work correctly (with or without explicit).

I have included a sample file below with comments on what I see in `emacs -q`


class Bob
{
    // string is `font-lock-type-face', Bob is `font-lock-function-name-face'
    Bob( string bob );
    // string and Bob are not fontified (though I sometimes see string fontified as a type)
    Bob( string );
    // string is `font-lock-variable-name-face', Bob is `font-lock-type-face'
    explicit Bob( string );
    // string is `font-lock-type-face', Bob is `font-lock-function-name-face'
    explicit Bob( string, string );
};

[-- Attachment #2: constructors.pdf --]
[-- Type: application/pdf, Size: 33105 bytes --]

[-- Attachment #3: Type: text/plain, Size: 1340 bytes --]



Thanks,
Ivan

In GNU Emacs 24.3.50.1 (i386-apple-darwin12.5.0, NS apple-appkit-1187.39)
of 2013-09-16 on ivanandres-MacBookPro
Bzr revision: 114309 rgm@gnu.org-20130916192145-tr5qrfdmhcq3a563
Windowing system distributor `Apple', version 10.3.1187
Configured using:
`configure --with-ns --with-xml2'

Important settings:
  locale-coding-system: nil
  default enable-multibyte-characters: t

Major mode: C++/lah

Minor modes in effect:
  diff-auto-refine-mode: t
  eldoc-mode: t
  highlight-parentheses-mode: t
  hl-sexp-mode: t
  highlight-symbol-mode: t
  which-function-mode: t
  show-paren-mode: t
  msb-mode: t
  minibuffer-depth-indicate-mode: t
  global-hl-line-mode: t
  delete-selection-mode: t
  auto-image-file-mode: t
  auto-insert-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  shell-dirtrack-mode: t
  ido-everywhere: t
  global-visible-mark-mode: t
  visible-mark-mode: t
  gvol-mode: t
  recentf-mode: t
  desktop-save-mode: t
  drag-stuff-global-mode: t
  drag-stuff-mode: t
  tooltip-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
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t
  abbrev-mode: t

^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#15415: 24.3.50; c++-mode fontification for constructors is inconsistent
  2013-09-19  2:44 bug#15415: 24.3.50; c++-mode fontification for constructors is inconsistent Ivan Andrus
@ 2013-09-30  3:31 ` Ivan Andrus
  2013-10-12 20:45   ` Alan Mackenzie
  2013-10-19 15:18   ` Alan Mackenzie
  0 siblings, 2 replies; 5+ messages in thread
From: Ivan Andrus @ 2013-09-30  3:31 UTC (permalink / raw)
  To: 15415

Ivan Andrus <darthandrus@gmail.com> writes:

> Font locking of C++ constructors is somewhat inconsistent.  This is no
> doubt complicated by the fact that unlike other function declarations
> they "don't have a return type".
>
> When a single argument is not used but named, the constructor is not
> fontified (normally it's fontified with `font-lock-function-name-face').
> If the keyword explicit is used, then the argument type is fontified as
> a variable, and the constructor name is fontified as a type.  Perhaps
> interestingly, naming the parameter or adding another parameter causes
> fontification to work correctly (with or without explicit).
>
> I have included a sample file below with comments on what I see in `emacs -q`
>
>
> class Bob
> {
>     // string is `font-lock-type-face', Bob is `font-lock-function-name-face'
>     Bob( string bob );
>     // string and Bob are not fontified (though I sometimes see string fontified as a type)
>     Bob( string );
>     // string is `font-lock-variable-name-face', Bob is `font-lock-type-face'
>     explicit Bob( string );
>     // string is `font-lock-type-face', Bob is `font-lock-function-name-face'
>     explicit Bob( string, string );
> };

In fact, it's not just constructors that have this problem.  For example
the following function declaration:

string lookup( size_t ) const;

Removing const, or adding a name to the size_t parameter causes
fontification to work correctly.

-Ivan





^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#15415: 24.3.50; c++-mode fontification for constructors is inconsistent
  2013-09-30  3:31 ` Ivan Andrus
@ 2013-10-12 20:45   ` Alan Mackenzie
  2013-10-18 22:00     ` Ivan Andrus
  2013-10-19 15:18   ` Alan Mackenzie
  1 sibling, 1 reply; 5+ messages in thread
From: Alan Mackenzie @ 2013-10-12 20:45 UTC (permalink / raw)
  To: Ivan Andrus; +Cc: 15415

Hello, Ivan.

On Sun, Sep 29, 2013 at 09:31:13PM -0600, Ivan Andrus wrote:
> Ivan Andrus <darthandrus@gmail.com> writes:

> > Font locking of C++ constructors is somewhat inconsistent.  This is
> > no doubt complicated by the fact that unlike other function
> > declarations they "don't have a return type".

This is, indeed, problematic.

> > When a single argument is not used but named, the constructor is not
> > fontified (normally it's fontified with
> > `font-lock-function-name-face').  If the keyword explicit is used,
> > then the argument type is fontified as a variable, and the
> > constructor name is fontified as a type.  Perhaps interestingly,
> > naming the parameter or adding another parameter causes fontification
> > to work correctly (with or without explicit).

Yes.  The pertinent function, `c-forward-decl-or-cast-1', is somewhat
probablistic.  If it gets sufficient clues from the context, it gets
things right, otherwise it has to guess, and sometimes will get things
wrong, particularly in C++, which doesn't have a nice context-free
syntax.

> > I have included a sample file below with comments on what I see in
> > `emacs -q`


  > > class Bob
  > > {
  > >     // string is `font-lock-type-face', Bob is `font-lock-function-name-face'
1 > >     Bob( string bob );
  > >     // string and Bob are not fontified (though I sometimes see string fontified as a type)
2 > >     Bob( string );
  > >     // string is `font-lock-variable-name-face', Bob is `font-lock-type-face'
3 > >     explicit Bob( string );
  > >     // string is `font-lock-type-face', Bob is `font-lock-function-name-face'
4 > >     explicit Bob( string, string );
  > > };

> In fact, it's not just constructors that have this problem.  For
> example the following function declaration:

5 > string lookup( size_t ) const;

> Removing const, or adding a name to the size_t parameter causes
> fontification to work correctly.

Yes.

Of the lines of code you've illustrated, 1 and 4 were OK.  I've corrected
3 and 5, which were relatively simple.

2 is a problem, because it looks like a normal function call.  If the
identifier in the parentheses (here "string") can be positively
identified as a type (for example, some use elsewhere can only be a type,
or it's a standard type like "string") it gets fontified.  Otherwise,
it's assumed the construct is a function call.  It would no doubt be
possible to check that the enclosing braces are a class declaration, and
that "Bob" is the name of the class, but this would slow down the
fontification, probably by a lot.

Would you please try out the patch below, and let me know how it goes.
It is based on the current source in the bzr trunk.

Again, thanks for such a crisp and concise bug report.



=== modified file 'lisp/progmodes/cc-engine.el'
*** lisp/progmodes/cc-engine.el	2013-09-28 17:17:01 +0000
--- lisp/progmodes/cc-engine.el	2013-10-12 20:18:26 +0000
***************
*** 6917,6923 ****
  	  ;; can happen since we don't know if
  	  ;; `c-restricted-<>-arglists' will be correct inside the
  	  ;; arglist paren that gets entered.
! 	  c-parse-and-markup-<>-arglists)
  
        (goto-char id-start)
  
--- 6917,6925 ----
  	  ;; can happen since we don't know if
  	  ;; `c-restricted-<>-arglists' will be correct inside the
  	  ;; arglist paren that gets entered.
! 	  c-parse-and-markup-<>-arglists
! 	  ;; Start of the identifier for which `got-identifier' was set.
! 	  name-start)
  
        (goto-char id-start)
  
***************
*** 6935,6941 ****
  			;; If the third submatch matches in C++ then
  			;; we're looking at an identifier that's a
  			;; prefix only if it specifies a member pointer.
! 			(when (setq got-identifier (c-forward-name))
  			  (if (looking-at "\\(::\\)")
  			      ;; We only check for a trailing "::" and
  			      ;; let the "*" that should follow be
--- 6937,6945 ----
  			;; If the third submatch matches in C++ then
  			;; we're looking at an identifier that's a
  			;; prefix only if it specifies a member pointer.
! 			(when (progn (setq pos (point))
! 				     (setq got-identifier (c-forward-name)))
! 			  (setq name-start pos)
  			  (if (looking-at "\\(::\\)")
  			      ;; We only check for a trailing "::" and
  			      ;; let the "*" that should follow be
***************
*** 6961,6967 ****
        ;; Skip over an identifier.
        (or got-identifier
  	  (and (looking-at c-identifier-start)
! 	       (setq got-identifier (c-forward-name))))
  
        ;; Skip over type decl suffix operators.
        (while (if (looking-at c-type-decl-suffix-key)
--- 6965,6973 ----
        ;; Skip over an identifier.
        (or got-identifier
  	  (and (looking-at c-identifier-start)
! 	       (setq pos (point))
! 	       (setq got-identifier (c-forward-name))
! 	       (setq name-start pos)))
  
        ;; Skip over type decl suffix operators.
        (while (if (looking-at c-type-decl-suffix-key)
***************
*** 7052,7074 ****
  		;; declaration.
  		(throw 'at-decl-or-cast t))
  
- 	      (when (and got-parens
- 			 (not got-prefix)
- 			 (not got-suffix-after-parens)
- 			 (or backup-at-type
- 			     maybe-typeless
- 			     backup-maybe-typeless))
- 		;; Got a declaration of the form "foo bar (gnu);" where we've
- 		;; recognized "bar" as the type and "gnu" as the declarator.
- 		;; In this case it's however more likely that "bar" is the
- 		;; declarator and "gnu" a function argument or initializer (if
- 		;; `c-recognize-paren-inits' is set), since the parens around
- 		;; "gnu" would be superfluous if it's a declarator.  Shift the
- 		;; type one step backward.
- 		(c-fdoc-shift-type-backward)))
  
! 	  ;; Found no identifier.
  
  	  (if backup-at-type
  	      (progn
  
--- 7058,7084 ----
  		;; declaration.
  		(throw 'at-decl-or-cast t))
  
  
! 	       (when (and got-parens
! 			  (not got-prefix)
! 			  ;; (not got-suffix-after-parens)
! 			  (or backup-at-type
! 			      maybe-typeless
! 			      backup-maybe-typeless
! 			      (eq at-decl-or-cast t)
! 			      (save-excursion
! 				(goto-char name-start)
! 				(not (memq (c-forward-type) '(nil maybe))))))
! 		 ;; Got a declaration of the form "foo bar (gnu);" or "bar
! 		 ;; (gnu);" where we've recognized "bar" as the type and "gnu"
! 		 ;; as the declarator.  In this case it's however more likely
! 		 ;; that "bar" is the declarator and "gnu" a function argument
! 		 ;; or initializer (if `c-recognize-paren-inits' is set),
! 		 ;; since the parens around "gnu" would be superfluous if it's
! 		 ;; a declarator.  Shift the type one step backward.
! 		 (c-fdoc-shift-type-backward)))
  
+ 	   ;; Found no identifier.
  	  (if backup-at-type
  	      (progn
  




> -Ivan

-- 
Alan Mackenzie (Nuremberg, Germany).





^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#15415: 24.3.50; c++-mode fontification for constructors is inconsistent
  2013-10-12 20:45   ` Alan Mackenzie
@ 2013-10-18 22:00     ` Ivan Andrus
  0 siblings, 0 replies; 5+ messages in thread
From: Ivan Andrus @ 2013-10-18 22:00 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: 15415@debbugs.gnu.org

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

Oops, forgot to CC the bug.

I've been using the patch for a week now and haven't noticed any ill
effects.

Thanks,
Ivan


On Sat, Oct 12, 2013 at 2:45 PM, Alan Mackenzie <acm@muc.de> wrote:

> Hello, Ivan.
>
> On Sun, Sep 29, 2013 at 09:31:13PM -0600, Ivan Andrus wrote:
> > Ivan Andrus <darthandrus@gmail.com> writes:
>
> > > Font locking of C++ constructors is somewhat inconsistent.  This is
> > > no doubt complicated by the fact that unlike other function
> > > declarations they "don't have a return type".
>
> This is, indeed, problematic.
>
> > > When a single argument is not used but named, the constructor is not
> > > fontified (normally it's fontified with
> > > `font-lock-function-name-face').  If the keyword explicit is used,
> > > then the argument type is fontified as a variable, and the
> > > constructor name is fontified as a type.  Perhaps interestingly,
> > > naming the parameter or adding another parameter causes fontification
> > > to work correctly (with or without explicit).
>
> Yes.  The pertinent function, `c-forward-decl-or-cast-1', is somewhat
> probablistic.  If it gets sufficient clues from the context, it gets
> things right, otherwise it has to guess, and sometimes will get things
> wrong, particularly in C++, which doesn't have a nice context-free
> syntax.
>
> > > I have included a sample file below with comments on what I see in
> > > `emacs -q`
>
>
>   > > class Bob
>   > > {
>   > >     // string is `font-lock-type-face', Bob is
> `font-lock-function-name-face'
> 1 > >     Bob( string bob );
>   > >     // string and Bob are not fontified (though I sometimes see
> string fontified as a type)
> 2 > >     Bob( string );
>   > >     // string is `font-lock-variable-name-face', Bob is
> `font-lock-type-face'
> 3 > >     explicit Bob( string );
>   > >     // string is `font-lock-type-face', Bob is
> `font-lock-function-name-face'
> 4 > >     explicit Bob( string, string );
>   > > };
>
> > In fact, it's not just constructors that have this problem.  For
> > example the following function declaration:
>
> 5 > string lookup( size_t ) const;
>
> > Removing const, or adding a name to the size_t parameter causes
> > fontification to work correctly.
>
> Yes.
>
> Of the lines of code you've illustrated, 1 and 4 were OK.  I've corrected
> 3 and 5, which were relatively simple.
>
> 2 is a problem, because it looks like a normal function call.  If the
> identifier in the parentheses (here "string") can be positively
> identified as a type (for example, some use elsewhere can only be a type,
> or it's a standard type like "string") it gets fontified.  Otherwise,
> it's assumed the construct is a function call.  It would no doubt be
> possible to check that the enclosing braces are a class declaration, and
> that "Bob" is the name of the class, but this would slow down the
> fontification, probably by a lot.
>
> Would you please try out the patch below, and let me know how it goes.
> It is based on the current source in the bzr trunk.
>
> Again, thanks for such a crisp and concise bug report.
>
>
>
> === modified file 'lisp/progmodes/cc-engine.el'
> *** lisp/progmodes/cc-engine.el 2013-09-28 17:17:01 +0000
> --- lisp/progmodes/cc-engine.el 2013-10-12 20:18:26 +0000
> ***************
> *** 6917,6923 ****
>           ;; can happen since we don't know if
>           ;; `c-restricted-<>-arglists' will be correct inside the
>           ;; arglist paren that gets entered.
> !         c-parse-and-markup-<>-arglists)
>
>         (goto-char id-start)
>
> --- 6917,6925 ----
>           ;; can happen since we don't know if
>           ;; `c-restricted-<>-arglists' will be correct inside the
>           ;; arglist paren that gets entered.
> !         c-parse-and-markup-<>-arglists
> !         ;; Start of the identifier for which `got-identifier' was set.
> !         name-start)
>
>         (goto-char id-start)
>
> ***************
> *** 6935,6941 ****
>                         ;; If the third submatch matches in C++ then
>                         ;; we're looking at an identifier that's a
>                         ;; prefix only if it specifies a member pointer.
> !                       (when (setq got-identifier (c-forward-name))
>                           (if (looking-at "\\(::\\)")
>                               ;; We only check for a trailing "::" and
>                               ;; let the "*" that should follow be
> --- 6937,6945 ----
>                         ;; If the third submatch matches in C++ then
>                         ;; we're looking at an identifier that's a
>                         ;; prefix only if it specifies a member pointer.
> !                       (when (progn (setq pos (point))
> !                                    (setq got-identifier
> (c-forward-name)))
> !                         (setq name-start pos)
>                           (if (looking-at "\\(::\\)")
>                               ;; We only check for a trailing "::" and
>                               ;; let the "*" that should follow be
> ***************
> *** 6961,6967 ****
>         ;; Skip over an identifier.
>         (or got-identifier
>           (and (looking-at c-identifier-start)
> !              (setq got-identifier (c-forward-name))))
>
>         ;; Skip over type decl suffix operators.
>         (while (if (looking-at c-type-decl-suffix-key)
> --- 6965,6973 ----
>         ;; Skip over an identifier.
>         (or got-identifier
>           (and (looking-at c-identifier-start)
> !              (setq pos (point))
> !              (setq got-identifier (c-forward-name))
> !              (setq name-start pos)))
>
>         ;; Skip over type decl suffix operators.
>         (while (if (looking-at c-type-decl-suffix-key)
> ***************
> *** 7052,7074 ****
>                 ;; declaration.
>                 (throw 'at-decl-or-cast t))
>
> -             (when (and got-parens
> -                        (not got-prefix)
> -                        (not got-suffix-after-parens)
> -                        (or backup-at-type
> -                            maybe-typeless
> -                            backup-maybe-typeless))
> -               ;; Got a declaration of the form "foo bar (gnu);" where
> we've
> -               ;; recognized "bar" as the type and "gnu" as the
> declarator.
> -               ;; In this case it's however more likely that "bar" is the
> -               ;; declarator and "gnu" a function argument or initializer
> (if
> -               ;; `c-recognize-paren-inits' is set), since the parens
> around
> -               ;; "gnu" would be superfluous if it's a declarator.  Shift
> the
> -               ;; type one step backward.
> -               (c-fdoc-shift-type-backward)))
>
> !         ;; Found no identifier.
>
>           (if backup-at-type
>               (progn
>
> --- 7058,7084 ----
>                 ;; declaration.
>                 (throw 'at-decl-or-cast t))
>
>
> !              (when (and got-parens
> !                         (not got-prefix)
> !                         ;; (not got-suffix-after-parens)
> !                         (or backup-at-type
> !                             maybe-typeless
> !                             backup-maybe-typeless
> !                             (eq at-decl-or-cast t)
> !                             (save-excursion
> !                               (goto-char name-start)
> !                               (not (memq (c-forward-type) '(nil
> maybe))))))
> !                ;; Got a declaration of the form "foo bar (gnu);" or "bar
> !                ;; (gnu);" where we've recognized "bar" as the type and
> "gnu"
> !                ;; as the declarator.  In this case it's however more
> likely
> !                ;; that "bar" is the declarator and "gnu" a function
> argument
> !                ;; or initializer (if `c-recognize-paren-inits' is set),
> !                ;; since the parens around "gnu" would be superfluous if
> it's
> !                ;; a declarator.  Shift the type one step backward.
> !                (c-fdoc-shift-type-backward)))
>
> +          ;; Found no identifier.
>           (if backup-at-type
>               (progn
>
>
>
>
>
> > -Ivan
>
> --
> Alan Mackenzie (Nuremberg, Germany).
>

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#15415: 24.3.50; c++-mode fontification for constructors is inconsistent
  2013-09-30  3:31 ` Ivan Andrus
  2013-10-12 20:45   ` Alan Mackenzie
@ 2013-10-19 15:18   ` Alan Mackenzie
  1 sibling, 0 replies; 5+ messages in thread
From: Alan Mackenzie @ 2013-10-19 15:18 UTC (permalink / raw)
  To: 15415-done

Bug fixed.

-- 
Alan Mackenzie (Nuremberg, Germany).





^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-10-19 15:18 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-19  2:44 bug#15415: 24.3.50; c++-mode fontification for constructors is inconsistent Ivan Andrus
2013-09-30  3:31 ` Ivan Andrus
2013-10-12 20:45   ` Alan Mackenzie
2013-10-18 22:00     ` Ivan Andrus
2013-10-19 15:18   ` Alan Mackenzie

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