On Tue, Jan 29, 2019 at 6:40 PM <gsquel...@mozilla.com> wrote:
> 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.
>
Oh but I think it will actually. :-) clang-format has a heuristic that
makes it put upper-case macros before function definition return types on
their own lines (e.g. see how our NS_IMETHODIMP macro is treated.)
> 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.
>
Interesting idea! I wonder if we are at a point in our adoption curve of
Rust that we should worry about how weird our C++ code looks for the
majority of developers yet... I suppose doing this would mean reserving
only five characters at the beginning of function names for the "return
type".
> 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
>
--
Ehsan
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
