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

Mon Feb 4 12:01:28 PST 2019

This might be a bit of a strange take, but as an outside user of LLVM, my
personal request would be that uses of `auto` be loosely predicated on not
breaking doxygen too badly/frequently. (I surmise this essentially means
discouraging `auto` when there are member functions being called from it
later to do interesting work, but I leave that for others to debate and
decide).

I know doxygen already has many other similar failure modes (such as
templates and auto-generated content, macros), but I thought offering this
view might provide some objectivity to the "more readable" question, as it
attempts to connect the definition of readability to a specific tooling
question ("is the type available to a locally-syntactic tool with a
particular purpose?") instead of subjective aesthetics (such as whether
auto in lambda is more or less pretty).

Of course, that still leaves many edge cases: for example, I found
http://llvm.org/doxygen/IRBuilder_8h_source.html#l02247 while searching for
an example for this email:
2247  if (const auto *CI = dyn_cast<ConstantInt>(OffsetValue))
2248  IsOffsetZero = CI->isZero();
2249

This is a case where auto is generally accepted (because the type is
obvious in the dyn_cast), but since doxygen doesn't know about that
convention, it is unable to find the documentation for the `isZero` call.

Anyways, my 2¢, from a lurker.

On Sun, Feb 3, 2019 at 1:31 PM Stephen Kelly via llvm-dev <
llvm-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? I'm getting the impression the topic should be dropped, but
> maybe I'm missing something?
>
>
> Thanks,
>
> Stephen.
>
>
> _______________________________________________
> 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/cfe-dev/attachments/20190204/0a0da884/attachment.html>