[PATCH] D113666: [flang][CodeGen] Transform `fir.emboxchar` to a sequence of LLVM MLIR

[Andrzej Warzynski via Phabricator via llvm-commits]

awarzynski added inline comments.


================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:1408
+
+    mlir::Location loc = emboxChar.getLoc();
+    mlir::Type llvmStructTy = convertType(emboxChar.getType());
----------------
mehdi_amini wrote:
> awarzynski wrote:
> > clementval wrote:
> > > `getLoc()` is almost 100% of the time `auto` in the LLVM/MLIR code base. 
> > Almost:
> > ```
> > grep  -iEr "Location getLoc" mlir/ | wc -l
> >        9
> > ```
> > But `auto` is indeed more popular. Let me revert this.
> > getLoc() is almost 100% of the time auto in the LLVM/MLIR code base.
> 
> I don't think so.
> I would use almost always `Location`: auto does not improve readability is enough for me to not use it (also it could be an `SMLoc` in some context) . When I use auto when writing code it is mostly out of laziness / convenience, but we should optimize for other to read code instead of the ease to write it.
> 
> I don't know how you grepped, but mine is:
> 
> ```
> $ git grep getLoc\( mlir | grep "loc =" | grep "auto" | wc -l
> 130
> $ git grep getLoc\( mlir | grep "loc =" | grep -v "auto" | wc -l
> 237
> ```
> 
> So much more 
Thanks for checking! Clearly I wasn't awake when `grepping`. 

> we should optimize for other to read code instead of the ease to write it.

Agreed. Will update before merging.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113666