Changes in LLVM code to satisfy Use auto Type Deduction to Make Code More Readable rule in LLVM coding standards.
Diff is produced by prototype check that is similar to enchantments made by malcolm.parsons in D27166 (modernize-use-auto).
Paths
| Differential D27652
Using auto for calls to cast-like functions to cover LLVM style guide. AcceptedPublic Authored by staronj on Dec 10 2016, 5:21 AM.
Details
Summary Changes in LLVM code to satisfy Use auto Type Deduction to Make Code More Readable rule in LLVM coding standards. Diff is produced by prototype check that is similar to enchantments made by malcolm.parsons in D27166 (modernize-use-auto).
Diff Detail Event Timelinestaronj updated this object. Comment Actions It's reasonable application of Clang-tidy, but I think will be better to split fixes in several commits based on functionality. Did you also enable analysis of headers? Comment Actions Could you please clarify situation with check? Similar thing is implemented in D27166. Comment Actions Forgive me if this question is too general or has an obvious answer... I am wondering what the advantage is of using the auto type specifier in place of actual type specifiers in general. I certainly understand it when it comes to overly verbose iterator declarations and nested types, but I fail to see the advantage of applying such a change as widely as this. So I am not fundamentally opposed to this change, but I would like to understand the rationale a bit more clearly rather than seeing a patch that has no description other than "results of running clang-tidy on LLVM source". As a PowerPC back end developer I spend a lot of time in that code and as I look at this patch, I do not see it as an obvious improvement to the code that is currently there. That is not to say I see any changes that hinder readability since it appears to only be applied to calls where the the explicit template argument specifies the return type. Comment Actions
In general: none.
I agree this a poorly described patch.
From I saw in this patch, it is implementing this: http://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable : do use auto with initializers like cast<Foo>(...) or other places where the type is already obvious from the context Comment Actions
We had race condition in fixing this bug and Malcolm published his solution before us. The check is doing the same thing. Comment Actions
What is the good way to split it in your opinion? These are changes to only this one case (auto * I = dyn_cast<Instr> etc), so there is only one functionality. Comment Actions
Exactly. Having redundant type is 1) redundant 2) bugprone (someone could change single type when modyfying source code). Comment Actions
Yes, because it is largely automatic and the pattern is well identified, I'm fine with a single patch. But please update the description! staronj retitled this revision from Changes in LLVM after running llvm-use-auto-in-calls check. to Using auto for calls to cast-like functions to cover LLVM style guide..Dec 13 2016, 6:43 AM staronj updated this object. staronj edited edge metadata. This revision is now accepted and ready to land.Dec 13 2016, 8:32 AM Large DiffThis large diff affects 472 files. Files without inline comments have been collapsed. Expand All Files
Revision Contents
Diff 80998 |