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

[Peter Klausler via Phabricator via llvm-commits]

klausler added a comment.

In D77821#1979385 <https://reviews.llvm.org/D77821#1979385>, @jdoerfert wrote:

> 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.)


I can't figure out what you're trying to express, or what part of my comment was "arguably not true".  The Fortran 2018 grammar has some conventions that allow the names of nonterminals to be qualified with semantic restriction prefixes (4.1.3 for "scalar").  So a "scalar-int-constant-expr" is an "int-constant-expr" (R1031) that is semantically constrained to be scalar and constant (C1031) in the Fortran sense of a "constant expression" (10.1.12) and integer (C1008).  The f18 parse tree represents constraints like this with template classes (`Scalar<>`, `Integer<>`, &c.) that wrap a subtree with a representation of a check that is applied after parsing during the semantic analysis and validation of expressions and variable references.  The expressions and variable references in the parse tree are analyzed and validated after parsing has recognized a syntactically valid program, name resolution has created the symbols and scopes and types, and any necessary corrections to ambiguous parses have been applied to the parse tree.  The semantic analysis of expressions and variables generates (in the absence of errors) a strongly-typed internal representation of those objects, and attaches those representations to the parse tree so that the later semantic passes have access to them.  All compiler messages from all phases get buffered, then sorted in source position order and emitted later after all messages have been created.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77821