[llvm-dev] Change coding style for argument alignment?
David Blaikie via llvm-dev
llvm-dev at lists.llvm.org
Thu Dec 3 14:38:31 PST 2020
On Thu, Dec 3, 2020 at 1:59 PM Krzysztof Parzyszek via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> > (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)
>
>
>
> Hm. It was Consolas at 10pt. What were you seeing?
>
Ah, indeed. Seems Consolas isn't commonly available on Mac (
https://www.cssfontstack.com/Consolas ) so it renders with some other
(variable width) font, unfortunately. I do see it formatted correctly when
I use a Windows computer.
>
>
> --
>
> Krzysztof Parzyszek kparzysz at quicinc.com AI tools development
>
>
>
> *From:* David Blaikie <dblaikie at gmail.com>
> *Sent:* Thursday, December 3, 2020 3:45 PM
> *To:* Krzysztof Parzyszek <kparzysz at quicinc.com>
> *Cc:* llvm-dev <llvm-dev at lists.llvm.org>
> *Subject:* [EXT] Re: [llvm-dev] Change coding style for argument
> alignment?
>
>
>
> (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)
>
> On Thu, Dec 3, 2020 at 1:33 PM Krzysztof Parzyszek via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
> 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.
>
>
>
> In practice there are already various mixtures of formatting being used.
> For example:
>
>
>
> bool tailDuplicate(bool IsSimple,
>
> MachineBasicBlock *TailBB,
>
> MachineBasicBlock *ForcedLayoutPred,
>
> SmallVectorImpl<MachineBasicBlock *> &TDBBs,
>
> SmallVectorImpl<MachineInstr *> &Copies,
>
> SmallVectorImpl<MachineBasicBlock *> *CandidatePtr);
>
> void appendCopies(MachineBasicBlock *MBB,
>
> SmallVectorImpl<std::pair<Register, RegSubRegPair>>
> &CopyInfos,
>
> SmallVectorImpl<MachineInstr *> &Copies);
>
>
>
>
>
> 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)
>
>
>
> void removeDeadBlock(
>
> MachineBasicBlock *MBB,
>
> function_ref<void(MachineBasicBlock *)> *RemovalCallback = nullptr);
>
>
>
> These are 3 adjacent declarations in the same file
> (llvm/include/llvm/CodeGen/TailDuplicator.h).
>
>
>
>
>
> 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.
>
>
> 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)
>
> - Dave
>
>
>
>
> --
>
> Krzysztof Parzyszek kparzysz at quicinc.com AI tools development
>
>
>
> *From:* llvm-dev <llvm-dev-bounces at lists.llvm.org> *On Behalf Of *Nikita
> Popov via llvm-dev
> *Sent:* Thursday, December 3, 2020 3:01 PM
> *To:* llvm-dev <llvm-dev at lists.llvm.org>
> *Subject:* [EXT] [llvm-dev] Change coding style for argument alignment?
>
>
>
> Hi,
>
>
>
> LLVM currently uses a coding style where arguments and parameters need to
> be aligned to the opening parenthesis "(".
>
>
>
> someFunctionCall(Arg1, Arg2,
>
> Arg3, Arg4);
>
>
>
> This style guideline is unfortunate for a couple of reasons:
>
>
>
> 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.
>
>
>
> 2. It causes significant right-ward drift. Especially for declarations,
> it's somewhat common for code ending up like this...
>
>
>
> Foo SomeClassName::someMethodName(Bar &Arg1,
> Bar &Arg2,
> Bar &Arg3,
> Bar &Arg4) {
>
>
>
> ... 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.
>
>
>
> 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.
>
>
>
> Is there any chance that the style could be changed towards indenting (but
> not aligning) the following line instead?
>
>
>
> someFunctionCall(
>
> Arg1, Arg2, Arg3, Arg4);
>
>
>
> This is unaffected by renames, does not cause rightward drift and results
> in very predictable formatting.
>
>
>
> 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.
>
>
>
> Regards,
>
> Nikita
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20201203/b8ab3cf3/attachment.html>
More information about the llvm-dev
mailing list