[PATCH] D112899: Update fir.insert_on_range syntax to make the range more explicit (NFC)

Mehdi AMINI via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 2 16:13:33 PDT 2021


mehdi_amini added inline comments.


================
Comment at: flang/lib/Optimizer/Dialect/FIROps.cpp:1400
+      parser.parseCommaSeparatedList(
+          AsmParser::Delimiter::Square,
+          [&] { return parser.parseInteger(lbounds.emplace_back(0)); }) ||
----------------
mehdi_amini wrote:
> kiranchandramohan wrote:
> > Nit: The fortran array syntax uses `Paren`. Might be good to use that. Not a strong opinion. You can wait for other's comments.
> > i.e Fortran syntax is a(1,3) and not a[1,3].
> Happy to adjust as needed!
> 
> Something that would be nice to clarify in a doc for the FIR dialect in general are some principles around row/col major and whether indexing starts at 0 or 1, I'm frequently confused.
> Happy to adjust as needed!

Thinking about it, I would likely try be to consistently using `(` `)` for Fortran indexing (col-major, starts at 1) and square brackets for the C-like indexing.
That way the syntax is a bit more than sugar but provides a visual aid on the semantics of the indexing, WDYT?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112899



More information about the llvm-commits mailing list