bug#15415: 24.3.50; c++-mode fontification for constructors is inconsistent

[Alan Mackenzie]

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