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

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan 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:
> 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?
> 
That sounds good to me.
WDYT @schweitz @clementval?

I can help with submitting this to fir-dev.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112899