After dealing with bugs that only happen in PGO builds, I would be worried
about bugs in the code-formatting-rewriter. The value doesn't seem worth
the risk to me.
- jared
On Fri, Feb 17, 2017 at 5:44 PM, <gsquel...@mozilla.com> wrote:
> On Saturday, February 18, 2017 at 9:29:06 AM UTC+11, Bobby Holley wrote:
> > On Fri, Feb 17, 2017 at 2:18 PM, <gsqu...@mozilla.com> wrote:
> >
> > > Hi again Nick,
> > >
> > > Someone made me realize that I didn't fully read your message, sorry
> for
> > > that.
> > >
> > > I now see that as well as &&/||, you have grepped for other operators,
> and
> > > shown that the overwhelming usage is to put all of them at the end of
> lines!
> > >
> > > In light of this, and from what others here&elsewhere have discussed,
> I'm
> > > now thinking that we probably should indeed just update our coding
> style
> > > with whatever is used the most out there, and model our "Official
> > > Formatter" on it.
> > >
> > >
> > > Another thought, if technically possible:
> > > If our main repository is going to always be under the control of some
> > > official clang-format style, it should be possible for anybody to pull
> the
> > > repository, and use a different formatter locally with their favorite
> > > style. And when pushing, their modified code could be automatically
> > > reformatted with the official formatter -- Everybody feels good when
> > > programming! :-)
> > >
> >
> > Please no. That would make reviewing a nightmare.
>
> Not sure what your concern is:
> - People who stick with the official parser will only ever see that
> (pushing to mozreview will reformat everything to the official style).
> - People who prefer a different style at home will have to adapt to the
> different style when reviewing others' code, or when their code gets review
> comments in a different style -- the formatter shouldn't change things so
> wildly that it would be so hard to find the corresponding location in their
> local repo.
>
> So it's everybody's choice to avoid a nightmare, or deal with it. ;-)
>
>
> > > Cheers,
> > > Gerald
> > >
> > >
> > > On Friday, February 17, 2017 at 5:16:42 PM UTC+11, Nicholas Nethercote
> > > wrote:
> > > > I personally have a strong preference for operators at the end of
> lines.
> > > > The codebase seems to agree with me, judging by some rough grepping
> > > ('fff'
> > > > is an alias I have that's roughly equivalent to rgrep):
> > > >
> > > > $ fff "&&$" | wc -l
> > > > 28907
> > > > $ fff "^ *&&" | wc -l
> > > > 3751
> > > >
> > > > $ fff "||$" | wc -l
> > > > 26429
> > > > $ fff "^ *||" | wc -l
> > > > 2977
> > > >
> > > > $ fff " =$" | wc -l
> > > > 39379
> > > > $ fff "^ *= " | wc -l
> > > > 629
> > > >
> > > > $ fff " +$" | wc -l
> > > > 31909
> > > > $ fff "^ *+$" | wc -l
> > > > 491
> > > >
> > > > $ fff " -$" | wc -l
> > > > 2083
> > > > $ fff "^ *-$" | wc -l
> > > > 52
> > > >
> > > > $ fff " ==$" | wc -l
> > > > 1501
> > > > $ fff "^ *== " | wc -l
> > > > 161
> > > >
> > > > $ fff " !=$" | wc -l
> > > > 699
> > > > $ fff "^ *!= " | wc -l
> > > > 129
> > > >
> > > > They are rough regexps and probably have some false positives, but
> the
> > > > numbers aren't even close; operators at the end of the line clearly
> > > > dominate.
> > > >
> > > > I will conform for the greater good but silently weep inside every
> time I
> > > > see it.
> > > >
> > > > Nick
> > > >
> > > > On Fri, Feb 17, 2017 at 8:47 AM, <gsqu...@mozilla.com> wrote:
> > > >
> > > > > Question of the day:
> > > > > When breaking overlong expressions, should &&/|| go at the end or
> the
> > > > > beginning of the line?
> > > > >
> > > > > TL;DR: Coding style says 'end', I&others think we should change it
> to
> > > > > 'beginning' for better clarity, and consistency with other
> operators.
> > > > >
> > > > >
> > > > > Our coding style reads:
> > > > > "Break long conditions after && and || logical connectives. See
> below
> > > for
> > > > > the rule for other operators." [1]
> > > > > """
> > > > > Overlong expressions not joined by && and || should break so the
> > > operator
> > > > > starts on the second line and starts in the same column as the
> start
> > > of the
> > > > > expression in the first line. This applies to ?:, binary arithmetic
> > > > > operators including +, and member-of operators (in particular the .
> > > > > operator in JavaScript, see the Rationale).
> > > > >
> > > > > Rationale: operator at the front of the continuation line makes for
> > > faster
> > > > > visual scanning, because there is no need to read to end of line.
> Also
> > > > > there exists a context-sensitive keyword hazard in JavaScript; see
> bug
> > > > > 442099, comment 19, which can be avoided by putting . at the start
> of a
> > > > > continuation line in long member expression.
> > > > > """ [2]
> > > > >
> > > > >
> > > > > I initially focused on the rationale, so I thought *all* operators
> > > should
> > > > > go at the front of the line.
> > > > >
> > > > > But it seems I've been living a lie!
> > > > > &&/|| should apparently be at the end, while other operators (in
> some
> > > > > situations) should be at the beginning.
> > > > >
> > > > >
> > > > > Now I personally think this just doesn't make sense:
> > > > > - Why the distinction between &&/|| and other operators?
> > > > > - Why would the excellent rationale not apply to &&/||?
> > > > > - Pedantically, the style talks about 'expression *not* joined by
> > > &&/||,
> > > > > but what about expression that *are* joined by &&/||? (Undefined
> > > Behavior!)
> > > > >
> > > > > Based on that, I believe &&/|| should be made consistent with *all*
> > > > > operators, and go at the beginning of lines, aligned with the first
> > > operand
> > > > > above.
> > > > >
> > > > > And therefore I would propose the following changes to the coding
> > > style:
> > > > > - Remove the lonely &&/|| sentence at [1].
> > > > > - Rephrase the first sentence at [2] to something like: "Overlong
> > > > > expressions should break so that the operator starts on the
> following
> > > line,
> > > > > in the same column as the first operand for that operator. This
> > > applies to
> > > > > all binary operators, including member-of operators (in particular
> the
> > > .
> > > > > operator in JavaScript, see the Rationale), and extends to ?: where
> > > the 2nd
> > > > > and third operands should be on separate lines and start in the
> same
> > > column
> > > > > as the first operand."
> > > > > - Keep the rationale at [2].
> > > > >
> > > > > Also, I think we should add something about where to break
> expressions
> > > > > with operators of differing precedences, something like: "Overlong
> > > > > expressions containing operators of differing precedences should
> first
> > > be
> > > > > broken at the operator of lowest precedence. E.g.: 'a+b*c' should
> be
> > > split
> > > > > at '+' before '*'"
> > > > >
> > > > >
> > > > > A bit more context:
> > > > > Looking at the history of the coding style page, a certain
> "Brendan"
> > > wrote
> > > > > that section in August 2009 [3], shortly after a discussion here
> [4]
> > > that
> > > > > seemed to focus on the dot operator in Javascript. In that
> discussion,
> > > > > &&/|| appear in examples at the end of lines and nobody talks about
> > > that
> > > > > (because it was not the main subject, and/or everybody agreed with
> it?)
> > > > >
> > > > > Discuss!
> > > > >
> > > > >
> > > > > [1] https://developer.mozilla.org/en-US/docs/Mozilla/Developer_
> > > > > guide/Coding_Style#Control_Structures
> > > > > [2] https://developer.mozilla.org/en-US/docs/Mozilla/Developer_
> > > > > guide/Coding_Style#Operators
> > > > > [3] https://developer.mozilla.org/en-US/docs/Mozilla/Developer_
> > > > > guide/Coding_Style$compare?locale=en-US&to=7315&from=7314
> > > > > [4] https://groups.google.com/d/msg/mozilla.dev.platform/
> > > > > Ji9lxlLCYME/zabUmQI9S-sJ
> > > > > _______________________________________________
> > > > > dev-platform mailing list
> > > > > dev-pl...@lists.mozilla.org
> > > > > https://lists.mozilla.org/listinfo/dev-platform
> > > _______________________________________________
> > > dev-platform mailing list
> > > dev-pl...@lists.mozilla.org
> > > https://lists.mozilla.org/listinfo/dev-platform
> > >
>
> _______________________________________________
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
