1786510 - C++ style guide: `// static` comment for implementations for static methods?

Status: NEW

(Core :: General, task)

Description

[Kagami Rosylight [:saschanaz] (they/them)]

There are several cases of this:

• // static: https://searchfox.org/mozilla-central/search?q=%5E%2F%2F+static%24&path=&case=false&regexp=true
• /* static */ : https://searchfox.org/mozilla-central/search?q=%2F*+static+*%2F&path=&case=false&regexp=false

But not all static method implementations have this. Should this be in the style guide, or does it already require one?

Comment 1

[Kagami Rosylight [:saschanaz] (they/them)]

Hi bholley, is this something you have an answer for? Thanks!

Comment 2

[Andrew McCreight [:mccr8]]

My vague memory is that initially /* static */ was required by the formatter despite people preferring // static, but some work was done to make the latter work, too. Unfortunately I can't find any related discussion on dev-platform so I might be mistaken.

Comment 3

[Andrew McCreight [:mccr8]]

I managed to find bug 1523969, which seems to be what I remembered. The first comment links to the dev-platform discussion.

Comment 4

[Kagami Rosylight [:saschanaz] (they/them)]

Thanks! My specific point here is more about whether having that comment at all or not, though.

Comment 5

[Andrew McCreight [:mccr8]]

Oh, sorry, I misunderstood.

Comment 6

[Bobby Holley (:bholley)]

Is this something that clang-format is capable of generating? What percentage of our static methods are currently annotated?

Comment 7

[Kagami Rosylight [:saschanaz] (they/them)]

I'm not sure about clang-format, and given that nothing forces the existence of the comment I believe some significant amount but I have no data. (I can't imagine how to get that data without implementing a linter.)

Comment 8

[Bobby Holley (:bholley)]

I think the comments are sometimes useful, and so I don't really want to forbid them. But I also don't think this is important enough to invest non-trivial resources into linting for if the tools don't exist already. If that's the case, my guidance is to leave this unspecified.