On Wednesday, January 30, 2019 at 9:57:02 AM UTC+11, Ehsan Akhgari wrote:
> On Mon, Jan 28, 2019 at 7:10 PM <gsqu...@mozilla.com> wrote:
> 
> > Just a thought: Would it be worth considering a blank macro, e.g.:
> > static void foo();
> > DECLARED_STATIC void foo() {...}
> >
> > On top of not being confused with other comments around, it could be
> > clang-checked so it's never wrong. (And maybe eventually enforced, like
> > MOZ_IMPLICIT is.)
> >
> 
> Yeah, that could be a future alternative, but it would require someone to
> do the hard work of implementing the required static analysis and landing
> it.  I think Ryan's proposal will probably simplify that process somewhat
> by making it possible to mass-replace a bunch of "// static" comments with
> "DECLARED_STATIC" or some such, but I don't want to hold the good solution
> here for a perfect solution in the future.  I would personally be OK for
> these two to happen incrementally.
> 
> Would you mind filing a bug for this please?
> 
> Thanks,
> Ehsan

Thank you Ehsan. Yes, a phased approach would definitely be the way to go.
https://bugzilla.mozilla.org/show_bug.cgi?id=1523793

I realize now that this doesn't help at all with the issue of valuable 
horizontal real-estate! Sorry for the tangent.

One (partly tongue-in-cheek) solution to make the important function name more 
prominent is to use trailing return types:
`auto Foo::Bar() -> void {`
Note that this is more easily greppable when searching for function names.
And it looks more like Rust.

To help with spacing, the `DECLARED_...` macros could be placed at the end (if 
possible?) :
`void Foo::Bar() DECLARED_STATIC {`
`auto Foo::Bar() -> void DECLARED_STATIC {`
But this feels uglier to me.

Cheers,
Gerald

> > On Tuesday, January 29, 2019 at 10:27:17 AM UTC+11, Ryan Hunt wrote:
> > > Yeah, personally I have found them be useful and don't have an issue
> > with keeping
> > > them. I just wasn't sure if that was a common experience.
> > >
> > > So for converting from C-style to C++-style, that would be:
> > >
> > > /* static */ void Foo::Bar() {
> > >  ...
> > > }
> > >
> > > // static
> > > void Foo::Bar() {
> > >  ...
> > > }
> > >
> > > I think that would be good. My one concern would be the presence of
> > other C++-style
> > > comments before the method definition. For example [1].
> > >
> > > Ideally documentation like that should go in the header by the method
> > declaration, but I
> > > have no idea if we consistently do that.
> > >
> > > [1]
> > https://hg.mozilla.org/mozilla-central/file/e4b9b1084292/layout/generic/nsFrame.cpp#l1023
> > >
> > > Thanks,
> > > Ryan
> > >
> > > ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> > > On Monday, January 28, 2019 12:51 PM, Ehsan Akhgari <ehsan...@gmail.com>
> > wrote:
> > >
> > > > This is indeed one of the cases where the reformat has made things
> > worse.  I think as a couple of people have already said, we'll find that
> > some people do find these annotations useful, even if they're not always
> > consistently present.
> > > >
> > > > The path to least resistance for addressing this problem may be to
> > convert these into C++-style comments and therefore moving them into their
> > own lines.  Would you be OK with that?
> > > >
> > > > Thanks,
> > > > Ehsan
> > > >
> > > > On Fri, Jan 25, 2019 at 11:49 PM Ryan Hunt <r...@eqrion.net> wrote:
> > > >
> > > >> Hi all,
> > > >>
> > > >> Quick C++ style question.
> > > >>
> > > >> A common pattern in Gecko is for method definitions to have a comment
> > with the
> > > >> 'static' or 'virtual' qualification.
> > > >>
> > > >> Before the reformat, the comment would be on it's own separate line
> > [1]. Now
> > > >> it's on the main line of the definition [2].
> > > >>
> > > >> For example:
> > > >>
> > > >> /* static */ void
> > > >> Foo::Bar() {
> > > >>   ...
> > > >> }
> > > >>
> > > >> vs.
> > > >>
> > > >> /* static */ void Foo::Bar() {
> > > >>   ...
> > > >> }
> > > >>
> > > >> Personally I think this now takes too much horizontal space from the
> > main
> > > >> definition, and would prefer it to be either on its own line or just
> > removed.
> > > >>
> > > >> Does anyone have an opinion on whether we still want these comments?
> > And if so
> > > >> whether it makes sense to move them back to their own line.
> > > >>
> > > >> (My ulterior motive is that sublime text's indexer started failing to
> > find
> > > >>  these definitions after the reformat, but that should be fixed
> > regardless)
> > > >>
> > > >> If you're interested in what removing these would entail, I wrote a
> > regex to
> > > >> make the change [3].
> > > >>
> > > >> Thanks,
> > > >> Ryan
> > > >>
> > > >> [1]
> > https://hg.mozilla.org/mozilla-central/file/0348d472115d/layout/generic/nsFrame.cpp#l1759
> > > >> [2]
> > https://hg.mozilla.org/mozilla-central/file/e4b9b1084292/layout/generic/nsFrame.cpp#l1756
> > > >> [3]
> > https://hg.mozilla.org/try/rev/31ab3e466b6f15dcdbb1aee47edabc7c358b86f2
> > > >>
> > > >
> > > > --
> > > > Ehsan
> >
> 
> 
> -- 
> Ehsan

_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Reply via email to