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

[Valentin Clement via Phabricator via llvm-commits]

clementval accepted this revision.
clementval added inline comments.


================
Comment at: flang/lib/Optimizer/Dialect/FIROps.cpp:1400
+      parser.parseCommaSeparatedList(
+          AsmParser::Delimiter::Square,
+          [&] { return parser.parseInteger(lbounds.emplace_back(0)); }) ||
----------------
kiranchandramohan wrote:
> 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.
+1 to use Fortran syntax. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112899