[llvm-dev] [cfe-dev] RFC: Modernizing our use of auto

Mon Feb 4 12:15:53 PST 2019

On Sun, Feb 3, 2019 at 10:31 AM Stephen Kelly via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

>
> On 03/02/2019 17:59, Mehdi AMINI wrote:
>
>
>
> On Sun, Feb 3, 2019 at 6:50 AM Stephen Kelly via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> On 31/12/2018 04:54, Chris Lattner via llvm-dev wrote:
>> >> Do those uses conform to the guide? If they don't, then should the
>> guide be updated? Are the types there 'obvious’?
>> >
>> > If/when we revise the policy, then it would make sense for
>> non-conforming uses of auto to be changed.  However, I don’t think that
>> actually making a widespread change would be high priority...
>> >
>> >> How did all of those uses get into the codebase? Does it indicate that
>> the guide is not followed, or does it indicate that the guide is too
>> subjective, or that maybe the 'the type must be obvios' guide does not
>> reflect the 'reality at the coalface' anymore? Should those uses of auto be
>> changed?
>> >
>> > My understanding is that there has been no widely understood or
>> accepted policy, so different coders and reviewers are doing different
>> things.
>>
>> One of the things which has no consensus here is whether 'auto' may be
>> used in lambdas (using c++-14).
>
>
> Under the current guidelines, my understanding is that nothing prevents to
> use auto in lambda in order to "make the code more readable" when "the type
> is already obvious from the context" or "when the type would have been
> abstracted away anyways, often behind a container’s typedef such as
> std::vector<T>::iterator".
>
>
> Some people seem to have objections to use of auto with range-for loops
> too.
> There doesn't seem to be consensus on what is 'readable'. Some people
> claim
> strongly that 'the type must be obvious. There even seems to be consensus
> on
> that phrase, though it doesn't seem to actually apply - We have lots of
> uses of
> auto with AST Matchers for example, and the type of the matcher is not
> obvious
> in that code. We have lots of similar 'the type is not "obvious"' code
> using auto.
>
> And yet,
>
>  if (const auto *TSI = D->getTypeSourceInfo())
>
> draws review comments that it must be changed.
>
>
> We don't need to update the guideline on auto to be able to use auto in
> lambda as soon as c++14 is available.
>
>
> That seems to depend on the reviewer, which means the code and the reviews
> will be inconsistent.
>
>
>
>
>
>> This feature was celebrated as a big
>> feature which gets unlocked by migrating to toolchains which provide
>> that feature:
>>
>>   https://groups.google.com/forum/#!msg/llvm-dev/0VkIuhn10nE/QZ5FwYEmHAAJ
>>
>> So, does this need a guideline update?
>>
>> Is there consistency in celbrating that but writing 'remove all use of
>> auto from this file' in reviews?
>>
>> If there's no consensus and no consistency, what does that mean for the
>> code?
>>
>> Is
>>
>>    if (const auto *TSI = D->getTypeSourceInfo())
>>
>> ok?
>>
>> Some reviewers say 'no'. What is the consensus and how is that expressed
>> in the guidelines?
>>
>> Does anyone have any interest in making the guidelines more clear on
>> this?
>>
>> I have made several proposals, and at least Chris agreed that something
>> should be improved, but then he left the discussion.
>>
>> Does anyone else think that something can be improved? Is anyone willing
>> to read and comment on my proposal and get a change to the guidelines
>> committed?
>>
>
> I think that multiple people read your proposal and gave feedback on
> Phabricator that mandates a revision (for instance for-range loop). Also in
> this topic I believe some feedback was given that rewording in order to
> remove ambiguity is always good, as long as the "spirit" of the current
> rule as it is written is preserved.
> So my take on the subject is that we're waiting on a new revision of your
> patch?
>
>
> We deliberately took the discussion off Phab and onto the mailing list
> to try to get more-fruitful discussion. For example in
>
>
>
> http://clang-developers.42468.n3.nabble.com/Re-llvm-dev-RFC-Modernizing-our-use-of-auto-td4063365.html#a4063424
>
>
> I suggested that 'New guidelines should...' and then wrote some
> content. If we don't agree on 'what new guidelines should do', then
> perhaps it is not time for a Phab patch yet?
>
>
> Thanks for responding to that mail! :) I responded to you, but the
> discussion again did not progress. Is there just not enough interest
> in this?
>

That'd be my take on it - that some amount of readability varies depending
on the reader (& will then vary depending on the reviewer, since they are
the reader). Yeah, some subprojects of LLVM (LLD, LLDB are noteworthy here)
have more esoteric/specialized styles than the rest of the LLVM project
(but even within LLVM proper, different areas have some variations - maybe
just due to history, not having been cleaned up for more modern (naming,
style, C++ usage, etc) conventions).

Yes, it's a source of some friction when an existing LLVM developer (or a
newcomer) moves into a different part of the project and writes what they
believe are idiomatic - but finds a different local style requested - this
is especially sub-optimal for newcomers where the friction is already
fairly high to get involved.

But it's hard to muster enough buy-in to change much of that, to be honest.

- Dave

> I'm getting the impression the topic should be dropped, but
> maybe I'm missing something?
>
>
> Thanks,
>
> Stephen.
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190204/d4292d38/attachment.html>