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

[George Burgess IV via cfe-dev]

> I think people are too eager to use `auto` because it is easy to write
but it makes the types substantially harder for the reader to understand

I'm probably the Nth person to ask this, but what keeps us from promoting
the use of a clang-tidy-powered tool that basically emits fixits of
s/auto/actual_type/?

It sounds like we're pretty constrained in where we can use auto (which I'm
completely in favor of, in no small part because of our pervasive use of
types spelled *Ref); a tool that's able to do that replacement and
correctly ignore trivially-okay `auto`s >90% of the time sounds like it'd
make both sides of this:

> I think people are too eager to use `auto` because it is easy to write
but it makes the types substantially harder for the reader to understand.

happy.

Naturally, this gets tricky as we move to c++ versions that allow auto in
more places, and is iffy in general in e.g. type-dependent contexts, but I
didn't say 100% accuracy ;)

On Tue, Dec 4, 2018 at 2:02 AM Chandler Carruth via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> On Wed, Nov 28, 2018 at 6:25 PM Chris Lattner via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
>> Generally no IMO, because the cases that produce optional are not obvious.
>>
>
> Just to say, +1 from me too.
>
>
>>
>> > * Can we use auto in c++14 lambda arguments with llvm::find_if(C,
>> [](const auto& i) { ... }) for example?
>> > * We need to use auto for structured bindings, if those are used.
>>
>> These both get into “yes, if the type is contextually obvious”.  I’m not
>> sure how to specify this though.
>>
>> Perhaps the rule came be rewritten more generally, as in “auto should
>> generally only be used when the inferred type is obvious from context, e.g.
>> as a result of a cast like dyn_cast or as a result of constructing a value
>> with a constructor containing a type name.”?
>>
>
> Strongly agree.
>
> I understand it may not be as precise and unambiguous as people would
> like, I still think this is the correct core rule: there needs to be *some*
> reason why the type is contextually obvious.
>
> And honestly, I'm very happy erring on the side of writing out the type
> name. I think people are too eager to use `auto` because it is easy to
> write but it makes the types substantially harder for the reader to
> understand.
>
> > In the case that came up in review for me, the code I submitted is
>> >
>> > template <typename BaseT, typename DerivedT>
>> > void registerIfNodeMatcher(...) {
>> >  auto NodeKind = ASTNodeKind::getFromNodeKind<DerivedT>();
>> >  if (!NodeKind.isNone())
>> >    NodeCtors[NodeKind] = std::make_pair(MatcherName, Descriptor);
>> > }
>> >
>> > but it was rejected as unreadable and I was given a link to the coding
>> guidelines.
>>
>> I agree, this seems overly strict.
>>
>
> I mean maybe. But looking at the example, I don't have a clue what type
> `NodeKind` is. I think this is borderline, and I think its fine to be
> somewhat subjective based on the reviewer that is more familiar with the
> code and APIs in question.
>
>
>> > I'd also like to update them so that
>> >
>> > llvm::Optional<std::pair<std::string, MatcherCtor>>
>> > getNodeConstructorType(ASTNodeKind targetType) {
>> >  auto const &ctors = RegistryData->nodeConstructors();
>> >  auto it = llvm::find_if(
>> >      ctors, [targetType](const NodeConstructorMap::value_type &ctor) {
>> >        return ctor.first.isSame(targetType);
>> >      });
>> >  if (it == ctors.end())
>> >    return llvm::None;
>> >  return it->second;
>> > }
>> >
>> > is acceptable. The `auto it` is already acceptable, but the `auto
>> const& ctors` depends on an interpretation of the guideline and was
>> rejected in the review I submitted.
>>
>> This seems like a step too far from me.  I don’t know what
>> nodeConstructors is here at all, and you’ve erased all type information
>> about what you’re dealing with.
>>
>
> Strongly agree. The `auto it` continues to seem fine, but the `ctors` here
> seems super confusing to leave off all type information.
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://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/20181204/8da894f7/attachment.html>