[PATCH] D120897: [flang] Remove 'using namespace mlir;` from header files

[Andrzej Warzynski via Phabricator via llvm-commits]

awarzynski added inline comments.


================
Comment at: flang/include/flang/Optimizer/Dialect/FIROpsSupport.h:20
 inline bool nonVolatileLoad(mlir::Operation *op) {
-  if (auto load = dyn_cast<fir::LoadOp>(op))
+  if (auto load = llvm::dyn_cast<fir::LoadOp>(op))
     return !load->getAttr("volatile");
----------------
clementval wrote:
> clementval wrote:
> > awarzynski wrote:
> > > clementval wrote:
> > > > it's probably `mlir::dyn_cast` here. 
> > > `mlir::dyn_cast` is in fact `llvm::dyn_cast`. See https://github.com/llvm/llvm-project/blob/1e082a4a9c2d5b3dba06561bc3a3764c4cdc0d25/mlir/include/mlir/Support/LLVM.h#L82
> > I know but in the code base we tend to use `mlir::` so it would be nice to have homogeneity here as well. 
> Just double checked in the flang codebase: 
> `mlir::dyn_cast` 93
> `llvm::dyn_cast` 0
> 
> I know but in the code base we tend to use mlir:: so it would be nice to have homogeneity here as well.

Not a fan, but that's a very good reason to use `mlir::dyn_cast` here 👍🏻  

> Just double checked in the flang codebase:
> mlir::dyn_cast 93
> llvm::dyn_cast 0

I also found 45 instances of `dyn_cast` (i.e. without namespace qualifiers).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120897