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