[flang-commits] [flang] [flang] Fix diagnostic order for invalid coarray dimension declarations (PR #219282)

[Caroline Newcombe via flang-commits]

cenewcombe wrote:

I'm adding my review to try to keep this work moving along, with the caveat that it's outside my usual area.

I'd like to question whether the new call-site opt-out is the right shape. #215886 did two things: it added the start/`emitAtStart` machinery to `WithMessageParser`, changing the anchoring for every user (including, I think, many that don't need/shouldn't use it?), and it added one new call site (the type-bound procedure binding parser at `Fortran-parsers.cpp:570`).

Would it be simpler to invert the flag instead — restore the failure-point default and make start-anchoring opt-in at the one site that demonstrably needed it?

```
// basic-parsers.h
constexpr WithMessageParser(MessageFixedText t, PA p, bool atStart = false)
    : text_{t}, parser_{p}, emitAtStart_{atStart} {}
...
      emitAtStart = emitAtStart_;
...
  const bool emitAtStart_{false};
...
template <typename PA>
inline constexpr auto withMessageAtStart(MessageFixedText msg, PA parser) {
  return WithMessageParser{msg, parser, /*atStart=*/true};
}
```
and
```
// Fortran-parsers.cpp:570 — the CONTAINS site opts in
        withMessageAtStart(
            "expected a type-bound procedure binding (PROCEDURE, GENERIC, or FINAL) after CONTAINS"_err_en_US,
```

`Fortran-parsers.cpp:737` then needs no change — the TypeDeclarationStmt ordering fixes itself.

Either way, I'd suggest adding a lit test to pin the behavior.

https://github.com/llvm/llvm-project/pull/219282