[PATCH] D111288: [fir] Add data flow optimization pass

Mehdi AMINI via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 8 19:55:37 PDT 2021


mehdi_amini added a comment.

In D111288#3052799 <https://reviews.llvm.org/D111288#3052799>, @schweitz wrote:

> We've made a decision a long time ago to use `auto` routinely and let the compiler type inference.
>
> Here is an actual type from the front end that might help in understanding why fully elaborated types do not improve readability.

Right, our coding guidelines are indeed recommending to use auto to improve readability, and the example of type you show here is exactly why we have this guideline: auto can greatly improve readability.

But there are plenty of cases where the improvement isn't clear to me, look at for example the case at hand here:

  for (auto &loadOp : loadOps) {

vs:

  for (ReadOp loadOp : loadOps) {

Is the `auto` improving readability here? If not why should we use it then?

In this particular case it is even worse: to me the auto is actively misleading:

1. the variable is named "loadOp" but isn't a `LoadOp` and is `ReadOp` instead.
2. The code takes a reference to the op, which makes me think it isn't an `op` since these are value-based class.

This is same for using `auto op` where we could read `Operation *op`, etc.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111288/new/

https://reviews.llvm.org/D111288



More information about the llvm-commits mailing list