[PATCH] D54877: Update coding guidelines regarding use of `auto`

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 26 20:52:10 PST 2018


Well I agree, i also think we should have clear guidelines when possible.
But “when possible” is itself subjective. We can’t cover every possible
scenario because it would be impossible to remember all the rules, so
instead we use general guidelines. So we have to have a balance between
rules that are useful while not specifying every detail of what the code
should look like, and then in code review there can be a second line of
defense for reviewers to point out issues.


Fwiw, I feel like this patch is a response to comments pointed out by
reviewers in
https://reviews.llvm.org/D54405, and I think when multiple people have
concerns about something, the correct response is to simply address the
concerns.  I’m not opposed to modifying the rules around auto, but I don’t
think it should be used as a way to avoid addressing reviewer feedback.


On Mon, Nov 26, 2018 at 8:35 PM Mehdi AMINI via Phabricator <
reviews at reviews.llvm.org> wrote:

> mehdi_amini added a comment.
>
> In D54877#1309017 <https://reviews.llvm.org/D54877#1309017>, @zturner
> wrote:
>
> > In D54877#1308855 <https://reviews.llvm.org/D54877#1308855>, @zturner
> wrote:
> >
> > >
> >
> >
> > at the end of the day, comments that arise in code review should take
> precedence over what the style guide does or doesn't say when it concerns
> readability, since it's an actual account of someone stating that they do
> or don't find something readable, and that's what we're trying to optimize
> for.
>
>
> FWIW as another data point: I have a totally opposed view on the subject:
> I rather have clear guidelines *when possible*. Otherwise it is the realm
> of inconsistency and instability: one reviewers makes you change towards a
> prefered idiom while the next reviewer  will prefer another one.
> Just because like you wrote before (with a twist): " because there is only
> 1 of this reviewers and there are many other people".
>
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D54877/new/
>
> https://reviews.llvm.org/D54877
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181126/3c775c3d/attachment.html>


More information about the llvm-commits mailing list