<div dir="ltr">Agree, no change in coding standard should or would ever be accompanied by a wholesale change. It's more just agreeing on what clang-format can automatically do for us, and then everyone using clang-format. Note that if you use the git clang-format extension (or an equivalent svn one if one exists) it only formats the lines in your diff, not the full content of each file you touch. So this is o nly ever happening to lines that were dirty anyway as a result of your CL</div><br><div class="gmail_quote"><div dir="ltr">On Fri, Dec 11, 2015 at 4:50 PM Jim Ingham <<a href="mailto:jingham@apple.com">jingham@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div>Note, it is pure speculation that this is a bug in clang-format based on where the : is placed.</div><div><br></div><div>I prefer the way we write it, because it places all the initializers on the same level, though I don’t feel so strongly about this one.</div><div><br></div><div>I would really rather not go through making wholesale changes like this unless there’s some compelling reason as it makes the history harder to trace by inserting random inessential checkins that you have to peer through. And this seems like make-work to me.<div><br></div></div><div></div></div><div style="word-wrap:break-word"><div>Jim</div></div><div style="word-wrap:break-word"><div><br><div><br></div><div><br><div><blockquote type="cite"><div>On Dec 11, 2015, at 4:40 PM, Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:</div><br><div><div dir="ltr">Hrmm, that's interesting. I guess I was mistaken in LLVM's rules then. Are we willing to accept this format in LLDB?<div><br></div><div><div style="font-family:Menlo;font-size:11px;line-height:normal;margin:0px">LabelDecl(DeclContext *DC, SourceLocation IdentL, IdentifierInfo *II,</div><div style="font-family:Menlo;font-size:11px;line-height:normal;margin:0px"> LabelStmt *S, SourceLocation StartL)</div><div style="font-family:Menlo;font-size:11px;line-height:normal;margin:0px"> : NamedDecl(Label, DC, IdentL, II),</div><div style="font-family:Menlo;font-size:11px;line-height:normal;margin:0px"> TheStmt(S),</div><div style="font-family:Menlo;font-size:11px;line-height:normal;margin:0px"> MSAsmNameResolved(false),</div><div style="font-family:Menlo;font-size:11px;line-height:normal;margin:0px"> LocStart(StartL) {}</div></div><div style="font-family:Menlo;font-size:11px;line-height:normal;margin:0px"><br></div><div style="font-family:Menlo;font-size:11px;line-height:normal;margin:0px"><span style="font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:13px;line-height:19.5px">If so that's less work I have to do on clang-format to get it up to spec.</span><br></div></div><br><div class="gmail_quote"><div dir="ltr">On Fri, Dec 11, 2015 at 4:34 PM Jim Ingham <<a href="mailto:jingham@apple.com" target="_blank">jingham@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><blockquote type="cite"><div>On Dec 11, 2015, at 2:42 PM, Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:</div><br><div><div dir="ltr">Yes, but as I mentioned, two things are still unsupported due to limitations in clang-format. They are return-type-on-new-line (only in declarations. clang-format supports it for definitions) and the constructor initializer list comma at the end (clang-format puts it at the beginning). <div><br></div></div></div></blockquote></div></div><div style="word-wrap:break-word"><div><blockquote type="cite"><div><div dir="ltr"><div>That said, the comma at the end of initializer list isn't documented on that page, and where we don't have a clearly documented rule, prefer the LLVM guidelines, so….<br></div></div></div></blockquote><div><br></div><div><div>That clang-format behavior seems weird to me, a quick scan through clang sources have the commas always at the end:</div><div><br></div><div><div style="margin:0px;font-size:11px;line-height:normal;font-family:Menlo"><div style="margin:0px;line-height:normal"> LabelDecl(DeclContext *DC, SourceLocation IdentL, IdentifierInfo *II,</div><div style="margin:0px;line-height:normal"> LabelStmt *S, SourceLocation StartL)</div><div style="margin:0px;line-height:normal"> : NamedDecl(Label, DC, IdentL, II),</div><div style="margin:0px;line-height:normal"> TheStmt(S),</div><div style="margin:0px;line-height:normal"> MSAsmNameResolved(false),</div><div style="margin:0px;line-height:normal"> LocStart(StartL) {}</div><div><br></div><div>etc. I can’t remember seeing code in clang that does:</div><div><br></div><div><div style="margin:0px;line-height:normal"> LabelDecl(DeclContext *DC, SourceLocation IdentL, IdentifierInfo *II,</div><div style="margin:0px;line-height:normal"> LabelStmt *S, SourceLocation StartL)</div><div style="margin:0px;line-height:normal"> : NamedDecl(Label, DC, IdentL, II)</div><div style="margin:0px;line-height:normal"> , TheStmt(S)</div><div style="margin:0px;line-height:normal"> , MSAsmNameResolved(false)</div><div style="margin:0px;line-height:normal"> , LocStart(StartL) {}</div></div><div><br></div></div></div></div>That looks really weird, since you have to look past the noise of those commas to see what you really care about. </div><div><br></div><div>We do differ in that we tend to write this:</div><div><br></div><div><div><div style="margin:0px;font-size:11px;line-height:normal;font-family:Menlo"><div style="margin:0px;line-height:normal"> LabelDecl(DeclContext *DC, SourceLocation IdentL, IdentifierInfo *II,</div><div style="margin:0px;line-height:normal"> LabelStmt *S, SourceLocation StartL) :</div><div style="margin:0px;line-height:normal"> NamedDecl(Label, DC, IdentL, II),</div><div style="margin:0px;line-height:normal"> TheStmt(S),</div><div style="margin:0px;line-height:normal"> MSAsmNameResolved(false),</div><div style="margin:0px;line-height:normal"> LocStart(StartL) {}</div><div style="margin:0px;line-height:normal"><br></div><div>with the colon at the end of the argument list. I don’t think any of this behavior is prescribed one way or the other in the actual coding conventions… Maybe there’s a bug in clang-format that if you put the : in the unexpected place the commas get moved to the wrong place as well? But the llvm coding conventions make no prescription about this at all. I don’t think our code layout should be based on clang format bugs and all. We certainly shouldn’t do wholesale reformats that just make history harder to read for that reason.</div></div></div></div></div><div style="word-wrap:break-word"><div><div><div style="margin:0px;font-size:11px;line-height:normal;font-family:Menlo"><div><br></div><div>Jim</div><div><br></div><div><br></div></div></div></div></div><div style="word-wrap:break-word"><div><blockquote type="cite"><div><div dir="ltr"><div><br><div class="gmail_quote"><div dir="ltr">On Fri, Dec 11, 2015 at 2:37 PM Todd Fiala <<a href="mailto:todd.fiala@gmail.com" target="_blank">todd.fiala@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Okay, but does the format match the LLDB-modified format with some kind of configuration file? We still need to match our guidelines here:<div><br></div><div><a href="http://lldb.llvm.org/lldb-coding-conventions.html" style="font-size:13px" target="_blank">http://lldb.llvm.org/lldb-coding-conventions.html</a><br></div><div><br></div><div>We can achieve that with a config file for it, right? (Maybe already existing, maybe in the lldb source tree already?)</div></div><div class="gmail_extra"></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Dec 11, 2015 at 2:35 PM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">With git you can already run "git clang-format". You just need `git-clang-format` to be in your PATH (it's under llvm/tools/clang). Not sure how to hook it into SVN</div><div><div><br><div class="gmail_quote"><div dir="ltr">On Fri, Dec 11, 2015 at 2:32 PM Eugene Zelenko <<a href="mailto:eugene.zelenko@gmail.com" target="_blank">eugene.zelenko@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">At least clang-format should be applied to all newly added files before commit.<br>
<br>
Eugene.<br>
<br>
On Fri, Dec 11, 2015 at 2:30 PM, Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br>
> Back on the topic of clang-format, what would it take to make clang-format a<br>
> regular part of peoples' workflows?<br>
><br>
> On Fri, Dec 11, 2015 at 2:27 PM Todd Fiala <<a href="mailto:todd.fiala@gmail.com" target="_blank">todd.fiala@gmail.com</a>> wrote:<br>
>><br>
>> Yep - sorry. I had been talking to Greg about this and misunderstood his<br>
>> comment on it. My mistake entirely. Kate and I just talked and she pointed<br>
>> me to your document, Jim.<br>
>><br>
>> The description was:<br>
>> where we had a clearly adhered to standard, keep it.<br>
>> whee we didn't, we adopted LLVM.<br>
>><br>
>> Sorry for rehashing!<br>
>><br>
>> -Todd<br>
>><br>
>> On Fri, Dec 11, 2015 at 2:12 PM, Jim Ingham <<a href="mailto:jingham@apple.com" target="_blank">jingham@apple.com</a>> wrote:<br>
>>><br>
>>><br>
>>> On Dec 11, 2015, at 2:01 PM, Todd Fiala via lldb-commits<br>
>>> <<a href="mailto:lldb-commits@lists.llvm.org" target="_blank">lldb-commits@lists.llvm.org</a>> wrote:<br>
>>><br>
>>><br>
>>><br>
>>> On Fri, Dec 11, 2015 at 1:59 PM, Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>><br>
>>> wrote:<br>
>>>><br>
>>>> On Fri, Dec 11, 2015 at 1:55 PM Todd Fiala via lldb-commits<br>
>>>> <<a href="mailto:lldb-commits@lists.llvm.org" target="_blank">lldb-commits@lists.llvm.org</a>> wrote:<br>
>>>>><br>
>>>>> Hey Eugene and Greg,<br>
>>>>><br>
>>>>> I thought we were doing spaces before the open parens in places like<br>
>>>>> this:<br>
>>>>><br>
>>>>> - BreakpointResolverSP resolver_sp(new BreakpointResolverFileLine<br>
>>>>> (NULL,<br>
>>>>> ...<br>
>>>>> + BreakpointResolverSP resolver_sp(new<br>
>>>>> BreakpointResolverFileLine(nullptr,<br>
>>>>><br>
>>>>> (see the removal of the space after BreakpointResolverFileLine from the<br>
>>>>> clang-tidy settings I presume).<br>
>>>>><br>
>>>>> Did I misunderstand that?<br>
>>>><br>
>>>><br>
>>>> This was officially removed from the coding standard some months ago,<br>
>>><br>
>>><br>
>>> Okay. Are we 100% in sync with LLVM coding standard guidelines? If so I<br>
>>> can just look there to see what we're supposed to be doing.<br>
>>><br>
>>><br>
>>> No, the differences between the lldb and llvm coding standards are<br>
>>> documented in:<br>
>>><br>
>>> <a href="http://lldb.llvm.org/lldb-coding-conventions.html" rel="noreferrer" target="_blank">http://lldb.llvm.org/lldb-coding-conventions.html</a><br>
>>><br>
>>> Jim<br>
>>><br>
>>><br>
>>>><br>
>>>> but not everyone has adopted this unfortunately. See r228860. It pains<br>
>>>> me to no end that we differ from LLVM, because it leads to exactly these<br>
>>>> type of problems where people aren't sure what the exact set of rules are.<br>
>>><br>
>>><br>
>>><br>
>>><br>
>>> --<br>
>>> -Todd<br>
>>> _______________________________________________<br>
>>> lldb-commits mailing list<br>
>>> <a href="mailto:lldb-commits@lists.llvm.org" target="_blank">lldb-commits@lists.llvm.org</a><br>
>>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits</a><br>
>>><br>
>>><br>
>><br>
>><br>
>><br>
>> --<br>
>> -Todd<br>
</blockquote></div>
</div></div></blockquote></div><br><br clear="all"><div><br></div></div><div class="gmail_extra">-- <br><div><div dir="ltr">-Todd</div></div>
</div></blockquote></div></div></div>
</div></blockquote></div></div></blockquote></div>
</div></blockquote></div><br></div></div></div></blockquote></div>