Re: FW: Font-lock misfontifies foo::bar in C++

Re: FW: Font-lock misfontifies foo::bar in C++

[Alan Mackenzie]

Hi, Simon!

On Mon, Jul 24, 2006 at 09:39:26AM +0100, Marshall, Simon wrote:
> Hi Alan, did this get forwarded to you too?
> Are you the right person to forward it to?

Yes.  Sorry, I've been very slow answering.  The canonical address for
CC Mode problems is <bug-cc-mode@gnu.org>.

> Thanks, Simon.



> -----Original Message-----
> From: Marshall, Simon 
> Sent: 22 June 2006 14:33
> To: 'Emacs Pretest Bug (emacs-pretest-bug@gnu.org)'
> Subject: Font-lock misfontifies foo::bar in C++
 
> src/emacs -Q foo.cpp
 
> In the foo.cpp buffer, insert the text:
 
> void foo::bar()	 // wrong - foo in font-lock-constant-face (otherwise ok)

The face for "foo" is deliberately set to c-reference-face-name (At
cc-fonts.el L667 (in (c-lang-defconst c-basic-matchers-before ....)).
c-reference-face-name is defined (earlier on in the file) as
c-label-face-name, which (in its turn) becomes font-lock-constant-face.

So this is quite deliberate.  I don't assert it's "right", for whatever
value of "right".  Maybe this could be described more clearly in the CC
Mode manual (on page "Faces").

> {
>   foo			;// ok - no fontification

What is this, syntactically?  Is it valid C++?

>  foo:			 // ok - foo in font-lock-constant-face (also used
> for labels)
>   foo::		;// ok - foo not fontified (but maybe could be as a type)

Is this syntactically valid C++?

>     foo::bar	 // wrong - foo in font-lock-constant-face

(See above.)

> The text "foo" in "foo::bar" is fontified in font-lock-constant-face, rather
> than font-lock-type-face.  In C++, at least, anything before a "::" is a
> namespace or class name.  Note that mis-fontification happens when the "b"
> of "bar" is typed.

> Sorry, no fix.  Cc-fonts.el is too scary for my diminishing elisp skills -
> and I'm wary of breaking something else anyway.

:-)  The fix, if such is needed, would be to redefine (defconst
c-reference-face ...) at cc-fonts.el L145.

> Simon.

-- 
Alan Mackenzie (Munich, Germany).

RE: FW: Font-lock misfontifies foo::bar in C++

[Marshall, Simon]

> > src/emacs -Q foo.cpp
>  
> > In the foo.cpp buffer, insert the text:
>  
> > void foo::bar()	 // wrong - foo in font-lock-constant-face
(otherwise ok)
> 
> The face for "foo" is deliberately set to 
> c-reference-face-name (At cc-fonts.el L667 (in 
> (c-lang-defconst c-basic-matchers-before ....)).
> c-reference-face-name is defined (earlier on in the file) as 
> c-label-face-name, which (in its turn) becomes 
> font-lock-constant-face.
> 
> So this is quite deliberate.  I don't assert it's "right", 
> for whatever value of "right".  Maybe this could be described 
> more clearly in the CC Mode manual (on page "Faces").

Hi Alan, why is c-reference-face-name used for foo in this context?  It
cannot be a reference, it is a type name.

> > {
> >   foo			;// ok - no fontification
> 
> What is this, syntactically?  Is it valid C++?

The ; was just to ensure emacs indented following lines.

> >   foo::		;// ok - foo not fontified (but maybe could be as a
type)
> 
> Is this syntactically valid C++?

Yes, if you ignore the ; (see above).  It makes no difference to
fontification.

> >     foo::bar	 // wrong - foo in font-lock-constant-face
> 
> (See above.)
> 
> > The text "foo" in "foo::bar" is fontified in font-lock-constant-face, 
> > rather than font-lock-type-face.  In C++, at least, anything before a 
> > "::" is a namespace or class name.  Note that mis-fontification happens
> > when the "b" of "bar" is typed.
> 
> :-)  The fix, if such is needed, would be to redefine 
> (defconst c-reference-face ...) at cc-fonts.el L145.

Currently, the variable c-reference-face-name is initialised to either
font-lock-reference-face (an alias for font-lock-constant-face which is
documented as to be used for constant and label names) or c-label-face-name
(which resolves to font-lock-label-face, font-lock-constant-face or
font-lock-reference-face).  

It is not documented, but on the face of it, c-reference-face-name suggests
it is not intended to be used for type names.  Am I misunderstanding the
purpose of it?

If you change the value of c-reference-face-name, won't it break other
things that use that face name for constants and labels?

Simon.

Re: FW: Font-lock misfontifies foo::bar in C++

[Alan Mackenzie]

Hi, Simon!

On Tue, Jul 25, 2006 at 09:48:45AM +0100, Marshall, Simon wrote:
> > > src/emacs -Q foo.cpp

> > > In the foo.cpp buffer, insert the text:

> > > void foo::bar()	 // wrong - foo in font-lock-constant-face
> (otherwise ok)

[ .... ]

> Hi Alan, why is c-reference-face-name used for foo in this context?  It
> cannot be a reference, it is a type name.

I emailed Martin Stjernholm, the author of the CC Mode font locking code,
and asked him about this.  Here is his answer:

#########################################################################

[ACM]:
> Martin, can you shed any light upon why "foo" in "foo::bar" is given
> c-reference-face-name?

Sure. "foo" doesn't get the type face since it isn't used as a type -
it's used as an identifier qualifier. Note that "foo" in a "foo::bar"
expression can be a namespace name too, which isn't a type at all.

So, put another way: The face is not chosen from what an identifier is
defined as but rather how it is used. Some examples:

Example 1:

    foo bar;

In this statement "foo" is used to specify the type of something.
Hence it gets the type face.

Example 2:

    ~foo();

This declares a destructor for the class foo. Here "foo" doesn't
specify the type of anything; it's only used in a special construct to
serve as the name of the destructor function. It therefore gets
font-lock-function-name-face.

Example 3:

    foo::bar

"foo" doesn't specify the type of anything here either. Its use is to
tell in which scope "bar" is declared. Hence it gets a face that is
different from the type face. I call this use "identifier qualifier".

To make this more obvious/precise, you could perhaps introduce another
face name variable, say c-qualifier-face-name, and make a suitable
mapping of it onto one of the existing font-lock faces. I don't think
it should be mapped to the type face though, because it would be used
for namespace names too. In lack of better alternatives I chose to map
it as I did.

(Btw, there's not necessarily a hard distinction between types and
constants in all languages. Again, it's (afaik) Pike that pushes the
limits in this regard. There a type name is nothing more than a class
that has been assigned to a constant.)
#########################################################################

[ .... ]

> Simon.

-- 
Alan.

RE: FW: Font-lock misfontifies foo::bar in C++

[Marshall, Simon]

> > Martin, can you shed any light upon why "foo" in "foo::bar" is given 
> > c-reference-face-name?
> 
> Sure. "foo" doesn't get the type face since it isn't used as 
> a type - it's used as an identifier qualifier. Note that 
> "foo" in a "foo::bar"
> expression can be a namespace name too, which isn't a type at all.

Alan, I already explained that it was either a type name or a namespace
name.
My point was that it definitely is *not* a reference (a constant or label).

> Example 3:
> 
>     foo::bar
> 
> "foo" doesn't specify the type of anything here either. Its 
> use is to tell in which scope "bar" is declared. Hence it 
> gets a face that is different from the type face. I call this 
> use "identifier qualifier".
> 
> To make this more obvious/precise, you could perhaps 
> introduce another face name variable, say 
> c-qualifier-face-name, and make a suitable mapping of it onto 
> one of the existing font-lock faces. I don't think it should 
> be mapped to the type face though, because it would be used 
> for namespace names too. In lack of better alternatives I 
> chose to map it as I did.

I think c-qualifier-face-name would be a good idea, but I think it odd to
pick a face for its value that is definitely wrong.  References are used for
constants and labels.  Why not pick a face that is usually right, eg, the
type face?

Perhaps a better solution would be to add a c++-font-lock-extra-qualifiers,
akin to c++-font-lock-extra-types, that is a list of qualifier names.  I
would suggest that, by default, these could be fontified in
font-lock-type-face, but perhaps you might like to add a
font-lock-qualifier-face to allow users (or modes) to specify what they
want.  For C++, this variable could default to '("std") or somesuch.  

So, by default, "foo::bar" would not cause "foo" to be fontified in any
face.  If the user wanted it to be fontified, they would modify
c++-font-lock-extra-types if it were a type or
c++-font-lock-extra-qualifiers if it were a namespace.

WDYT?  I'm not on either mailing list, so please CC: me on any reply.

Thanks, Simon.

Re: FW: Font-lock misfontifies foo::bar in C++

[Alan Mackenzie]

Morning, Simon!

On Mon, Aug 07, 2006 at 09:32:59AM +0100, Marshall, Simon wrote:
> > > Martin, can you shed any light upon why "foo" in "foo::bar" is given 
> > > c-reference-face-name?

> > Sure. "foo" doesn't get the type face since it isn't used as 
> > a type - it's used as an identifier qualifier. Note that 
> > "foo" in a "foo::bar"
> > expression can be a namespace name too, which isn't a type at all.

> Alan, I already explained that it was either a type name or a namespace
> name.
> My point was that it definitely is *not* a reference (a constant or label).

#########################################################################

OK.  Just to clarify where I think we are - In this:

    void foo::bar()

, "foo" gets font-lock-constant-face.

(i) We both agree that this face assignment is deliberate and
  documented: On the page "Faces" in the CC Mode manual, it says:
  * Name qualifiers and identifiers for scope constructs are
    fontified like labels.
  * Label identifiers get `font-lock-constant-face' if it exists,
    `font-lock-reference-face' otherwise.
(ii) You're saying that fontifying "foo::" like a label is bad.

#########################################################################

> > Example 3:

> >     foo::bar

> > "foo" doesn't specify the type of anything here either. Its 
> > use is to tell in which scope "bar" is declared. Hence it 
> > gets a face that is different from the type face. I call this 
> > use "identifier qualifier".

> > To make this more obvious/precise, you could perhaps 
> > introduce another face name variable, say 
> > c-qualifier-face-name, and make a suitable mapping of it onto 
> > one of the existing font-lock faces. I don't think it should 
> > be mapped to the type face though, because it would be used 
> > for namespace names too. In lack of better alternatives I 
> > chose to map it as I did.

> I think c-qualifier-face-name would be a good idea, but I think it odd to
> pick a face for its value that is definitely wrong.  References are used for
> constants and labels.  Why not pick a face that is usually right, eg, the
> type face?

I'm at a disadvantage here, because I don't feel at all strongly about
what the right face is for "foo::", as long as it's visually distinct.
In fact I'd never really noticed that it was font-lock-constant-face.

You're an expert on font-lock, though.  Why would f-l-type-face be any
better?  Martin feels quite strongly, that "foo::bar" is not using foo
"in a type context", and has referred to Pike (one of CC Mode's other
languages), where a constant declaration is a special case of a class
declaration.

> Perhaps a better solution would be to add a
> c++-font-lock-extra-qualifiers, akin to c++-font-lock-extra-types,
> that is a list of qualifier names.  I would suggest that, by default,
> these could be fontified in font-lock-type-face, but perhaps you might
> like to add a font-lock-qualifier-face to allow users (or modes) to
> specify what they want.  For C++, this variable could default to
> '("std") or somesuch.  

> So, by default, "foo::bar" would not cause "foo" to be fontified in any
> face.  If the user wanted it to be fontified, they would modify
> c++-font-lock-extra-types if it were a type or
> c++-font-lock-extra-qualifiers if it were a namespace.

> WDYT?

OK; maybe.  But that would definitely be a new feature for CC Mode 5.32,
not something to slip in to 5.31.n in a hurry.

As regards f-l-constant-face on "foo::":  I don't really want to change
it, since this would be a lot of work.  Changing the code in cc-fonts.el
and the text in the manual would be relatively quick and easy, but then
I'd have to update the regression test program 000tests.el, and
recalcuate all the "face files" in the test suite, diff them with the
existing versions, and commit them to the CVS repository.  There are 166
files.face involved.  I would surely also get complaints from people
who've got used to the existing fontification.

So why is f-l-constant-face so jarring here?  I can see that it's not
particularly appropriate, but what is there about it that makes it
repulsive and ghastly, that raises a calm competent hacker's hackles?
(This isn't a rhetorical question.) 

> I'm not on either mailing list, so please CC: me on any reply.

Not merely a CC, but a To: is what you get.  :-)

> Thanks, Simon.

-- 
Alan.


-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

RE: FW: Font-lock misfontifies foo::bar in C++

[Marshall, Simon]

> OK.  Just to clarify where I think we are - In this:
> 
>     void foo::bar()
> 
> , "foo" gets font-lock-constant-face.
> 
> (i) We both agree that this face assignment is deliberate and
>   documented: On the page "Faces" in the CC Mode manual, it says:
>   * Name qualifiers and identifiers for scope constructs are
>     fontified like labels.
>   * Label identifiers get `font-lock-constant-face' if it exists,
>     `font-lock-reference-face' otherwise.
> (ii) You're saying that fontifying "foo::" like a label is bad.
> 
> ##############################################################
> ###########
> 
> > > Example 3:
> 
> > >     foo::bar
> 
> > > "foo" doesn't specify the type of anything here either. Its 
> > > use is to tell in which scope "bar" is declared. Hence it 
> > > gets a face that is different from the type face. I call this 
> > > use "identifier qualifier".
> 
> > > To make this more obvious/precise, you could perhaps 
> > > introduce another face name variable, say 
> > > c-qualifier-face-name, and make a suitable mapping of it onto 
> > > one of the existing font-lock faces. I don't think it should 
> > > be mapped to the type face though, because it would be used 
> > > for namespace names too. In lack of better alternatives I 
> > > chose to map it as I did.
> 
> > I think c-qualifier-face-name would be a good idea, but I think it odd
to
> > pick a face for its value that is definitely wrong.  References are used
for
> > constants and labels.  Why not pick a face that is usually right, eg,
the
> > type face?
> 
> I'm at a disadvantage here, because I don't feel at all strongly about
> what the right face is for "foo::", as long as it's visually distinct.
> In fact I'd never really noticed that it was font-lock-constant-face.
> 
> You're an expert on font-lock, though.  Why would f-l-type-face be any
> better?  Martin feels quite strongly, that "foo::bar" is not using foo
> "in a type context", and has referred to Pike (one of CC Mode's other
> languages), where a constant declaration is a special case of a class
> declaration.

OK, let's take this qualifier thing at face value.  (In C++, "foo" in
"foo::bar" is a qualifier.  It is the name either of a type or a namespace.)

class CLASS;			// CLASS is in font-lock-type-face
namespace NAMESPACE;		// NAMESPACE is now in
font-lock-constant-face
void CORN::fubar();		// CORN is now in font-lock-constant-face

(In Emacs 19/20/21, CLASS, NAMESPACE and CORN are all in
font-lock-type-face.)

By the qualifier argument, CORN cannot be in the face used for type names,
because it is not used "in a type context".  But, by the same argument, it
cannot be in the face used for namespace names either, because it is not
used in a namespace context.  (a) Since CORN is fontified in the same face
as namespace names, your qualifier argument leads me to the conclusion that
the current behaviour is broken.  By the qualifier argument, if you really
wanted to distinguish between namespace names, type names and qualifiers,
you would have to use 3 distinct faces.

This leads me to my original bug report.  (b) The use of
font-lock-constant-face for either NAMESPACE or CORN is wrong, because that
face is for constants and labels.  That face is quite explicitly documented
that way.  

Therefore, by your (a) and my (b), you need to use a new face for NAMESPACE
and a new face for CORN.

To return to the qualifier argument.  The contexts are (1) a type name in a
type context, (2) a type name in a qualifier context, (3) a namespace name
in a namespace context, and (4) a namespace name in a qualifier context.
Currently, you distinguish between (1) and (2,3,4).  This is wrong
regardless of the faces used.  Personally, I do not see the value in
distinguishing between each of them (I hope you don't think that the
qualifier argument implies that you should!), or even between (1), (3) and
(2,4).  I don't buy the qualifier argument at all.  There would be too many
faces for one, and too little practical gain for another.

To return to my original report.  I originally made (1,2,3,4) be fontified
as a type name mostly for ease of implementation and performance, not
because I had a particular view of whether it was worth distinguishing
between them.  I think it would be useful to change this, so that (1,2) are
distinguished from (3,4).  But you shouldn't use font-lock-constant-face to
fontify a namespace name.  

That's why I say it would be better to fontify as a type name, at least
until type names and namespace names can be dealt with individually as I
suggest below.

(I don't see the relevance of Pike---this is just about C++.)

> > Perhaps a better solution would be to add a
> > c++-font-lock-extra-qualifiers, akin to c++-font-lock-extra-types,
> > that is a list of qualifier names.  I would suggest that, by default,
> > these could be fontified in font-lock-type-face, but perhaps you might
> > like to add a font-lock-qualifier-face to allow users (or modes) to
> > specify what they want.  For C++, this variable could default to
> > '("std") or somesuch.  
> 
> > So, by default, "foo::bar" would not cause "foo" to be fontified in any
> > face.  If the user wanted it to be fontified, they would modify
> > c++-font-lock-extra-types if it were a type or
> > c++-font-lock-extra-qualifiers if it were a namespace.
> 
> > WDYT?
> 
> OK; maybe.  But that would definitely be a new feature for CC 
> Mode 5.32, not something to slip in to 5.31.n in a hurry.
> 
> As regards f-l-constant-face on "foo::":  I don't really want to change
> it, since this would be a lot of work.  Changing the code in cc-fonts.el
> and the text in the manual would be relatively quick and easy, but then
> I'd have to update the regression test program 000tests.el, and
> recalcuate all the "face files" in the test suite, diff them with the
> existing versions, and commit them to the CVS repository.  There are 166
> files.face involved.  I would surely also get complaints from people
> who've got used to the existing fontification.

I don't recall any complaints back in the Emacs 19/20/21 days when it
fontified class names and namespace names as type names.  Maybe I'm being
presumptive, but I'm sure those who noticed type names and namespace names
now being fontified as constants/labels won't mind it going back to be as
type names.

> So why is f-l-constant-face so jarring here?  I can see that it's not
> particularly appropriate, but what is there about it that makes it
> repulsive and ghastly, that raises a calm competent hacker's hackles?
> (This isn't a rhetorical question.) 

I don't claim to be calm or competent, and I never said it was repulsive or
ghastly.  But, visual cues are not helpful if they are wrong, particularly
when they are something as fundamental as classes and namespaces are in C++.

The only thing that could raise my hackles is that I feel I'm just repeating
myself.  And this has been going on nearly 2 months.  I don't mind
resolution taking that long---nobody is paid to do this---it's the lack of
progress on such a simple issue that is frustrating.  But perhaps I share
some of the blaim for that.

Thanks, Simon.

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642