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

[Chris Lattner via llvm-dev]

On Dec 16, 2018, at 11:44 AM, Stephen Kelly via llvm-dev <llvm-dev at lists.llvm.org> wrote:
> 
> On 25/11/2018 14:43, Stephen Kelly via llvm-dev wrote:
>> However this is a proposal for more modern thinking regarding the permissiveness of auto in LLVM codebases.
>> Currently the rule on the use of auto is here:
> 
> Hi,
> 
> Thanks for the input on this topic, which has appeared here on the mailing list, on the phab review, and on IRC.
> 
> Almost all respondants generally expressed the claim "The type must be obvious if auto is used", though as I wrote before the guide uses auto in context that the type is not obvious:
> 
> for (const auto &Val : Container) { observe(Val); }
> 
> It seems that the respondants wish for 'almost never auto'. Fair enough, if the existing practice supports that.
> 
> There is one problem though, or rather ~56,000 problems. That's how many uses of auto there are in the entire codebase currently, according to someone on IRC.

I find this to be a helpful line of argument.  We should, as a community, decide what the right thing is regardless of the existing art.  As you say, the current patches going in have been arbitrary, so making an argument based on what is in the code base isn’t particularly informative.

> 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.

> How is a new contributor to react to any of that? What are the real criteria that we can use to determine where auto will cause a patch to be rejected? Does it only depend on who you get as a reviewer?

Right now, it is quite arbitrary.  This is a bug, not a feature.

> Here is a quote from this thread from Chris and supported by Chandler and Quentin at least:
> 
> > 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.”?
> 
> Is it reasonable to have that as a rule if there are ~12000 uses which break that rule?

Yes.  If you’d like to make progress on this, I think you should start by building consensus.  It seems like there is widespread enough acknowledgement that the current state of things is broken, but there is no concrete proposal for a coding standards change.  Please prepare a patch so we can discuss it.

-Chris