[llvm-dev] RFC: Modernizing our use of auto
Jameson Nash via llvm-dev
llvm-dev at lists.llvm.org
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
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();
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
>> > My understanding is that there has been no widely understood or
>> accepted policy, so different coders and reviewers are doing different
>> 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
> Some people seem to have objections to use of auto with range-for loops
> There doesn't seem to be consensus on what is 'readable'. Some people
> strongly that 'the type must be obvious. There even seems to be consensus
> 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
> 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:
>> 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
>> if (const auto *TSI = D->getTypeSourceInfo())
>> 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
>> 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
> 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
> We deliberately took the discussion off Phab and onto the mailing list
> to try to get more-fruitful discussion. For example in
> 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?
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-dev