Re: indentation uncertainty
On Feb 13, 2018, a...@gnu.org (Alfred M. Szmidt) wrote: >>> return rup->ru_utime.tv_sec*1000 + rup->ru_utime.tv_usec/1000 >>> + rup->ru_stime.tv_sec*1000 + rup->ru_stime.tv_usec/1000; >>> I'd consider this improper, since it puts the operator where it >>> doesn't belong >> Why does not belong IYHO? >> The coding standards recommend that the expression follows below it >> self. >Where? > The examples being quite canonical IMHO. I can't derive it from the canonical rules, I'm afraid. Care to spell out, quote or otherwise point to the rules you believe any of these break? return foo + bar; return foo + bar; foo = bar + baz; > Then they are doing it "wrong" -- or at least not following the coding > standards ... or maybe the coding standards don't say what you seem to think they do? > As for the disagreement, _I_ don't see it I got contradictory responses from two different people about the very point of contention that had been raised elsewhere. If you can't see the disagreement there... would you like to borrow my glasses? :-D -- Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Re: indentation uncertainty
>> return rup->ru_utime.tv_sec*1000 + rup->ru_utime.tv_usec/1000 >> + rup->ru_stime.tv_sec*1000 + rup->ru_stime.tv_usec/1000; >> I'd consider this improper, since it puts the operator where it >> doesn't belong >Why does not belong IYHO? > The coding standards recommend that the expression follows below it > self. Where? The examples being quite canonical IMHO. > That is, we don't write: > mode = inmode[j] == VOIDmode > || GET_MODE_SIZE (outmode[j]) > GET_MODE_SIZE (inmode[j]) > ? outmode[j] : inmode[j]; Actually, some people do break the line before the || and indent it just like that. That would show exactly the point of contention I've raised, if it weren't for the '?' on the next line, that places operators of different precedences at the same alignment. Then they are doing it "wrong" -- or at least not following the coding standards -- which is just fine too. >Now, given that there is disagreement even here, I wonder if this is the >proper forum to determine what the fix to the recommendation should be. >Should I take it elsewhere, or is this the right forum to debate what >the GNU general recommendation should be? > This is probobly the right forum, I would suggest maybe adding > addition examples, or maybe simply mentioning that the same rules > apply for sizeof/return/... Should I wait till we sort out the disagreement before proposing patches for the standard, or should I propose a patch that states what I think it was always meant to be? A patch is I think always welcome, specially if it can clarify or put less burden on the grey cells. As for the disagreement, _I_ don't see it but that does not mean that it isn't there, I think the standard in this regard is clear. But, things can always be made clearer if there is such disagreement, not to mention that the standards are just a recommendation -- one is free to apply them as a whole, parts or none.
Re: indentation uncertainty
On Feb 9, 2018, a...@gnu.org (Alfred M. Szmidt) wrote: >> return rup->ru_utime.tv_sec*1000 + rup->ru_utime.tv_usec/1000 >> + rup->ru_stime.tv_sec*1000 + rup->ru_stime.tv_usec/1000; >> I'd consider this improper, since it puts the operator where it >> doesn't belong >Why does not belong IYHO? > The coding standards recommend that the expression follows below it > self. Where? > That is, we don't write: > mode = inmode[j] == VOIDmode > || GET_MODE_SIZE (outmode[j]) > GET_MODE_SIZE (inmode[j]) > ? outmode[j] : inmode[j]; Actually, some people do break the line before the || and indent it just like that. That would show exactly the point of contention I've raised, if it weren't for the '?' on the next line, that places operators of different precedences at the same alignment. > I don't see the difference between "mode =", sizeof, or return. Indeed, there doesn't seem to be reason to distinguish between e.g.: return inmode[j] == VOIDmode || GET_MODE_SIZE (outmode[j]) > GET_MODE_SIZE (inmode[j]); or bool var = inmode[j] == VOIDmode || GET_MODE_SIZE (outmode[j]) > GET_MODE_SIZE (inmode[j]); > clarification cannot hurt. :-) :-) >Now, given that there is disagreement even here, I wonder if this is the >proper forum to determine what the fix to the recommendation should be. >Should I take it elsewhere, or is this the right forum to debate what >the GNU general recommendation should be? > This is probobly the right forum, I would suggest maybe adding > addition examples, or maybe simply mentioning that the same rules > apply for sizeof/return/... Should I wait till we sort out the disagreement before proposing patches for the standard, or should I propose a patch that states what I think it was always meant to be? -- Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Re: indentation uncertainty
>return rup->ru_utime.tv_sec*1000 + rup->ru_utime.tv_usec/1000 > + rup->ru_stime.tv_sec*1000 + rup->ru_stime.tv_usec/1000; > I'd consider this improper, since it puts the operator where it > doesn't belong Why does not belong IYHO? The coding standards recommend that the expression follows below it self. That is, we don't write: mode = inmode[j] == VOIDmode || GET_MODE_SIZE (outmode[j]) > GET_MODE_SIZE (inmode[j]) ? outmode[j] : inmode[j]; But rather position "|| ..." below the RHS part of the expression. Ignoring nesting, and with the extra parens. so that Emacs does the right thing this being the canonical "nice" example: mode = (inmode[j] == VOIDmode || GET_MODE_SIZE (outmode[j]) > GET_MODE_SIZE (inmode[j]) ? outmode[j] : inmode[j]); That is in the examples you gave, + being located directly under the return keyword is "improper", it should instead be located under rup->ru_utime.tv_sec. I don't see the difference between "mode =", sizeof, or return. Which is also why I don't see why clarification is needed, but on the other hand ... clarification cannot hurt. :-) Now, given that there is disagreement even here, I wonder if this is the proper forum to determine what the fix to the recommendation should be. Should I take it elsewhere, or is this the right forum to debate what the GNU general recommendation should be? This is probobly the right forum, I would suggest maybe adding addition examples, or maybe simply mentioning that the same rules apply for sizeof/return/...
Re: indentation uncertainty
On Feb 7, 2018, Rical Jasan wrote: > On 02/06/2018 08:26 PM, Alexandre Oliva wrote: >> Some uncertainly has come up during GCC patch review. > A reference to the debate would be nice. I didn't want to pollute the conversation about GNU standards with the position of any one person involved. It's just not relevant for the documentation of what the GNU general recommendation is. Individual projects can have their own narrower rules, if they find it important, but first we have to find what the general guidance is. > How does the GCC codebase typically handle such formatting? There are examples that some consider improper, and that's what started the conversation and brought me here. On Feb 7, 2018, a...@gnu.org (Alfred M. Szmidt) wrote: >return rup->ru_utime.tv_sec*1000 + rup->ru_utime.tv_usec/1000 > + rup->ru_stime.tv_sec*1000 + rup->ru_stime.tv_usec/1000; > I'd consider this improper, since it puts the operator where it > doesn't belong Why does not belong IYHO? >If any of them are, the addition, to the standard document, of the >snippet and rationale would be appreciated. > I am not sure if an addition is needed, the above example fit with the > proper and improper examples from the standard document. So far, what has become clear is that there is disagreement as to at least one of the cases, and that one happens to be the very contentious point in the GCC community, which suggests to me we could use clarification in the GNU standard. Now, given that there is disagreement even here, I wonder if this is the proper forum to determine what the fix to the recommendation should be. Should I take it elsewhere, or is this the right forum to debate what the GNU general recommendation should be? Thanks, -- Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
Re: indentation uncertainty
On 02/06/2018 08:26 PM, Alexandre Oliva wrote: > Hi, there, > > Some uncertainly has come up during GCC patch review. A reference to the debate would be nice. > I'd appreciate > some clarification on the indentation rules in the GNU coding > standards. The current document states: > > Insert extra parentheses so that Emacs will indent the code properly. To provide context from the "Formatting Your Source Code" section this example is taken from [0]: "The rest of this section gives our recommendations... We don’t think of these recommendations as requirements, because it causes no problems for users if two different programs have different formatting styles. But whatever style you use, please use it consistently, since a mixture of styles within one program tends to look ugly. If you are contributing changes to an existing program, please follow the style of that program." > For example, the following indentation looks nice if you do it by > hand, > > v = rup->ru_utime.tv_sec*1000 + rup->ru_utime.tv_usec/1000 > + rup->ru_stime.tv_sec*1000 + rup->ru_stime.tv_usec/1000; > > but Emacs would alter it. Adding a set of parentheses produces > something that looks equally nice, and which Emacs will preserve: > > v = (rup->ru_utime.tv_sec*1000 + rup->ru_utime.tv_usec/1000 > + rup->ru_stime.tv_sec*1000 + rup->ru_stime.tv_usec/1000); > > It's not clear whether the first code snippet is incorrectly indented, > or whether the only problem with it is that mechanical reindent will > then make it improper. Given the context of /recommended/ formatting, I would say the first code snippet is correctly indented. > Mechanical reindent would turn it into: > > v = rup->ru_utime.tv_sec*1000 + rup->ru_utime.tv_usec/1000 > + rup->ru_stime.tv_sec*1000 + rup->ru_stime.tv_usec/1000; > > which is improper, presumably because '=' and '+' are at the same > indentation level, in spite of being operators with different > precedence, contradicting the earlier rule: > > Try to avoid having two operators of different precedence at the same > level of indentation. I don't read the document as stating the non-parenthesized mechanical indentation is "improper" as in "wrong" (actually, the way the expression lines up is kind of nice, IMO), but it does _recommend_ keeping operators of different precedence at different levels. How does the GCC codebase typically handle such formatting? If there is a mixture, is there a clear majority? If not, then I would say take your pick; otherwise the example has already been set. Maybe consider a refactoring patch that settles the issue once and for all. ;) > Now, what if, instead of as assignment, we had a return statement, > without an '=' operator of higher precedence? Perhaps return could be considered an operator (of higher precedence) in this case? > Would any of the following 4 forms be deemed improper in GNU code? In my opinion/interpretation: > return rup->ru_utime.tv_sec*1000 + rup->ru_utime.tv_usec/1000 >+ rup->ru_stime.tv_sec*1000 + rup->ru_stime.tv_usec/1000; No. > return (rup->ru_utime.tv_sec*1000 + rup->ru_utime.tv_usec/1000 > + rup->ru_stime.tv_sec*1000 + rup->ru_stime.tv_usec/1000); No. > return rup->ru_utime.tv_sec*1000 + rup->ru_utime.tv_usec/1000 > + rup->ru_stime.tv_sec*1000 + rup->ru_stime.tv_usec/1000; No. Even if you were concerned with operator precedence, no two operators of different precedence are indented at the same level (I would consider the "operators" to be "return" and "+"), so you're still indented according to the recommendation in that regard, but there may be an issue with the fact the "+" is so far left of the beginning of the expression. I would defer to any project-based precedent here. > return > (rup->ru_utime.tv_sec*1000 + rup->ru_utime.tv_usec/1000 >+ rup->ru_stime.tv_sec*1000 + rup->ru_stime.tv_usec/1000); No. I could see this being particularly useful if something in the expression caused this to be an overly-long line. > If any of them are, the addition, to the standard document, of the > snippet and rationale would be appreciated. Again, some references to the debate would be nice, but I'm not sure clarification is absolutely necessary. Rical [0] https://www.gnu.org/prep/standards/standards.html#Formatting
Re: indentation uncertainty
Would any of the following 4 forms be deemed improper in GNU code? All of this is IMHO. return rup->ru_utime.tv_sec*1000 + rup->ru_utime.tv_usec/1000 + rup->ru_stime.tv_sec*1000 + rup->ru_stime.tv_usec/1000; I'd consider this improper, since Emacs will reindent it -- thus some parens are needed. return (rup->ru_utime.tv_sec*1000 + rup->ru_utime.tv_usec/1000 + rup->ru_stime.tv_sec*1000 + rup->ru_stime.tv_usec/1000); Fine, as this puts the operator where it belongs -- with the rest of the expression. return rup->ru_utime.tv_sec*1000 + rup->ru_utime.tv_usec/1000 + rup->ru_stime.tv_sec*1000 + rup->ru_stime.tv_usec/1000; I'd consider this improper, since it puts the operator where it doesn't belong (i.e. same as if you indent the example code in the GCS). This is also the result when indenting in Emacs. return (rup->ru_utime.tv_sec*1000 + rup->ru_utime.tv_usec/1000 + rup->ru_stime.tv_sec*1000 + rup->ru_stime.tv_usec/1000); Fine, since maybe the first line contains a very long identifier or that it makes it legible -- same for if and while. If any of them are, the addition, to the standard document, of the snippet and rationale would be appreciated. I am not sure if an addition is needed, the above example fit with the proper and improper examples from the standard document.
