Reported by: Ivan Andrus <darthandrus <at> gmail.com>
Date: Thu, 19 Sep 2013 02:46:01 UTC
Severity: normal
Found in version 24.3.50
Done: Alan Mackenzie <acm <at> muc.de>
Bug is archived. No further changes may be made.
Message #14 received at 15415 <at> debbugs.gnu.org (full text, mbox):
From: Ivan Andrus <darthandrus <at> gmail.com> To: Alan Mackenzie <acm <at> muc.de> Cc: "15415 <at> debbugs.gnu.org" <15415 <at> debbugs.gnu.org> Subject: Re: bug#15415: 24.3.50; c++-mode fontification for constructors is inconsistent Date: Fri, 18 Oct 2013 16:00:45 -0600
[Message part 1 (text/plain, inline)]
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 <at> muc.de> wrote:
> Hello, Ivan.
>
> On Sun, Sep 29, 2013 at 09:31:13PM -0600, Ivan Andrus wrote:
> > Ivan Andrus <darthandrus <at> 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).
>
[Message part 2 (text/html, inline)]
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.