[flang-commits] [flang] [flang][OpenMP] Properly resolve CRITICAL construct names (PR #205904)

[Sairudra More via flang-commits]

Saieiei wrote:

This looks correct to me, and the cleanup is a real improvement. Replacing the previous `Internal: no symbol found` / `CRITICAL argument should be a name` / `is not a valid locator` cascade with a single diagnostic, and resolving the lock symbol in `ResolveCriticalName`, both look right.

One optional consistency/severity question, not a blocker: OpenMP 5.2 §4 does reserve names beginning with `omp_` / `ompx_`, so the diagnostic is spec-backed. But under the same rule, ordinary declarations like `integer :: omp_c` / `logical :: omp_available` would also be non-conforming, and Flang appears to accept those today. So this currently enforces the prefix rule for `critical` names but not for ordinary declarations.

Given that, and given the compatibility concern in #205855, do we intentionally want this as a hard error here, or would a warning/portability diagnostic be a better fit until reserved-name checking is applied more uniformly? The existing `CRITICAL construct name ... conflicts with a previous declaration` case is also a warning, so I wanted to ask before approving the severity choice.


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