[PATCH] D77821: [Flang][OpenMP] Avoid abort when collapse clause value is negative

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In D77821#1975101 <https://reviews.llvm.org/D77821#1975101>, @klausler wrote:

> > As far as I can tell, the collapse argument is a `scalarIntConstantExpr` to the parser. So conceptually one could make it a `scalarPositiveIntConstantExpr` and issue an error early.
>
> Fortran has scalar-int-constant-expr but not scalar-positive-int-constant-expr as nonterminals.
>
> The f18 parser doesn't do any semantic checking in any case.


This is arguably not true. The OpenMP grammar does not restrict the the collapse value to be a constant, the description does *in the same sentence* it is restricted to be positive.
If you would not perform any semantic check you would allow an arbitrary expression here and check it later in sema. My argument now is that this does make as much sense as checking that it is a constant but not if it is positive.

> The various constrained nonterminals of Fortran are represented in the parse tree and checked later in expression semantics.

But apparently not before you access/use input not validated yet. Take the TODO that I quoted (which is above the method that now duplicates the semantic check for the collapse clause value).
It already noted that there is an implicit dependence here. Resolving that by duplicating checks seems conceptually the wrong approach. (You also now have signed values floating around even though you want conceptually unsigned values.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77821