[PATCH] D97073: [flang][fir] Update flang test tool support classes.

Valentin Clement via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 19 12:49:42 PST 2021


clementval added inline comments.


================
Comment at: flang/include/flang/Optimizer/Support/FIRContext.h:17
+
+#ifndef FORTRAN_OPTIMIZER_SUPPORT_FIRCONTEXT_H
+#define FORTRAN_OPTIMIZER_SUPPORT_FIRCONTEXT_H
----------------
schweitz wrote:
> clementval wrote:
> > I thing we get this warning quite often. Is there a difference between header guard in Flang and the one clang-tidy expect? 
> Yes, it seems to complain about every header file in flang/include/.
> 
> It looks like clang-tidy should be fine with this identifier spelling.
> 
> Tracking it here: https://github.com/flang-compiler/f18-llvm-project/issues/631
Ok


================
Comment at: flang/include/flang/Optimizer/Support/FIRContext.h:25
+class ModuleOp;
+}
+
----------------
schweitz wrote:
> clementval wrote:
> > Do you want to add `// namespace mlir`?
> Looks like a sharp edge between clang-format and clang-tidy. clang-format doesn't put it in if there is only 1 declaration. clang-tidy whines about it.
Yeah I guess it makes sense since it is so close it's still readable without adding noise. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97073



More information about the llvm-commits mailing list