<div dir="ltr"><div dir="ltr">(probably worth using a monospaced font when discussing indent styles in HTML email, otherwise it's a bit hard to tell what's being discussed)<br><br>On Thu, Dec 3, 2020 at 1:33 PM Krzysztof Parzyszek via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div lang="EN-US" style="overflow-wrap: break-word;">
<div class="gmail-m_8955184210845654265WordSection1">
<p class="MsoNormal">I support this proposal. I very much dislike the current function parameter formatting (in function declarations/definitions). It just adds a bunch of whitespace without improvement in readability.<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">In practice there are already various mixtures of formatting being used. For example:<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal"><span style="font-size:10pt"><font face="monospace"> bool tailDuplicate(bool IsSimple,<u></u><u></u></font></span></p>
<p class="MsoNormal"><span style="font-size:10pt"><font face="monospace"> MachineBasicBlock *TailBB,<u></u><u></u></font></span></p>
<p class="MsoNormal"><span style="font-size:10pt"><font face="monospace"> MachineBasicBlock *ForcedLayoutPred,<u></u><u></u></font></span></p>
<p class="MsoNormal"><span style="font-size:10pt"><font face="monospace"> SmallVectorImpl<MachineBasicBlock *> &TDBBs,<u></u><u></u></font></span></p>
<p class="MsoNormal"><span style="font-size:10pt"><font face="monospace"> SmallVectorImpl<MachineInstr *> &Copies,<u></u><u></u></font></span></p>
<p class="MsoNormal"><span style="font-size:10pt"><font face="monospace"> SmallVectorImpl<MachineBasicBlock *> *CandidatePtr);<u></u><u></u></font></span></p>
<p class="MsoNormal"><span style="font-size:10pt"><font face="monospace"> void appendCopies(MachineBasicBlock *MBB,<u></u><u></u></font></span></p>
<p class="MsoNormal"><span style="font-size:10pt"><font face="monospace"> SmallVectorImpl<std::pair<Register, RegSubRegPair>> &CopyInfos,<u></u><u></u></font></span></p>
<p class="MsoNormal"><span style="font-size:10pt"><font face="monospace"> SmallVectorImpl<MachineInstr *> &Copies);<u></u><u></u></font></span></p>
<p class="MsoNormal"><span style="font-size:10pt"><font face="monospace"><u></u> </font></span></p></div></div></blockquote><div><br></div><div>Looks like "appendCopies" is the odd one out here. Probably someone didn't reindent when the name was changed. Hmm, nope, it was checked in that way - but it doesn't match any style I think we've ever had, so it's not really an argument for/against any particular format - the author seems to have uesd right alignment to fit the long name. (possible that our style guide didn't or doesn't specify what to do when things are too long - but I think generally, the answer is what's done for removeDeadBlock: Fallback to a double indent from the start, and this is what clang-format does with its LLVM style interpretation)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div lang="EN-US" style="overflow-wrap: break-word;"><div class="gmail-m_8955184210845654265WordSection1"><p class="MsoNormal"><span style="font-size:10pt"><font face="monospace"><u></u></font></span></p>
<p class="MsoNormal"><span style="font-size:10pt"><font face="monospace"> void removeDeadBlock(<u></u><u></u></font></span></p>
<p class="MsoNormal"><span style="font-size:10pt"><font face="monospace"> MachineBasicBlock *MBB,<u></u><u></u></font></span></p>
<p class="MsoNormal"><span style="font-size:10pt"><font face="monospace"> function_ref<void(MachineBasicBlock *)> *RemovalCallback = nullptr);</font></span><u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">These are 3 adjacent declarations in the same file (llvm/include/llvm/CodeGen/TailDuplicator.h).<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">I don’t think that the alternative format would make call sites immune to renaming, since it can still cause fewer or more function arguments to fit in a given line of code. I still think it’s a worthwhile change in style.</p></div></div></blockquote><div><br>I don't have strong feelings either way - mostly of the opinion that it's a "clang-format and forget about it" sort of thing. (which isn't to say formatting issues can't rise to the level of "this is actively harmful to readability of the code" but this one doesn't rise to that level from my perspective at least)<br><br>- Dave<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div lang="EN-US" style="overflow-wrap: break-word;"><div class="gmail-m_8955184210845654265WordSection1"><p class="MsoNormal"><u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal"><span style="font-size:9pt;font-family:Consolas">-- </span>
<span style="font-size:9pt;font-family:Consolas"><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:8pt;font-family:Consolas">Krzysztof Parzyszek
<a href="mailto:kparzysz@quicinc.com" target="_blank"><span style="color:rgb(5,99,193)">kparzysz@quicinc.com</span></a> AI tools development<u></u><u></u></span></p>
<p class="MsoNormal"><u></u> <u></u></p>
<div style="border-right:none;border-bottom:none;border-left:none;border-top:1pt solid rgb(225,225,225);padding:3pt 0in 0in">
<p class="MsoNormal"><b>From:</b> llvm-dev <<a href="mailto:llvm-dev-bounces@lists.llvm.org" target="_blank">llvm-dev-bounces@lists.llvm.org</a>> <b>On Behalf Of
</b>Nikita Popov via llvm-dev<br>
<b>Sent:</b> Thursday, December 3, 2020 3:01 PM<br>
<b>To:</b> llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>><br>
<b>Subject:</b> [EXT] [llvm-dev] Change coding style for argument alignment?<u></u><u></u></p>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal">Hi,<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">LLVM currently uses a coding style where arguments and parameters need to be aligned to the opening parenthesis "(".<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<p class="MsoNormal"> someFunctionCall(Arg1, Arg2,<u></u><u></u></p>
<div>
<p class="MsoNormal"> Arg3, Arg4);<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">This style guideline is unfortunate for a couple of reasons:<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">1. If a function is renamed, it is necessary to also reindent the arguments at all call-sites. For functions with many or complex arguments, this may require significant reflow.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">2. It causes significant right-ward drift. Especially for declarations, it's somewhat common for code ending up like this...<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal"> Foo SomeClassName::someMethodName(Bar &Arg1,<br>
Bar &Arg2,<br>
Bar &Arg3,<br>
Bar &Arg4) {<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">... because there is just enough space to fit each argument individually, but still a need to break each one out on a separate line. Closure arguments are another common case of very awkward formatting.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">3. There are some arcane rules about when this is not the preferred style and you're supposed to indent arguments on the following line instead.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Is there any chance that the style could be changed towards indenting (but not aligning) the following line instead?<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal"> someFunctionCall(<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> Arg1, Arg2, Arg3, Arg4);<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">This is unaffected by renames, does not cause rightward drift and results in very predictable formatting.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Based on past discussions, LLVM seems to be open to improving coding style for the better, so I thought I'd give this a shot, as this is a continuous source of friction for me.<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Regards,<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">Nikita<u></u><u></u></p>
</div>
</div>
</div>
</div>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div></div>