On Mon, Jan 28, 2019 at 7:10 PM <gsquel...@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 > 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 > _______________________________________________ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > -- Ehsan _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform