[flang-commits] [flang] [flang][OpenMP] Implicit declarations of procedures in DECLARE_TARGET (PR #201935)

[via flang-commits]

================
@@ -9756,6 +9742,28 @@ void ResolveNamesVisitor::HandleProcedureName(
     Symbol::Flag flag, const parser::Name &name) {
   CHECK(flag == Symbol::Flag::Function || flag == Symbol::Flag::Subroutine);
   auto *symbol{FindSymbol(NonDerivedTypeScope(), name)};
+  // A symbol listed on OpenMP declare_target directive may be a variable
+  // or a procedure. If the directive is the first occurrence of the name,
+  // it will create an implicit declaration of an object (since the name
+  // is not used in a call at that location). If the name turns out to be
+  // that of a procedure, this is going to create a problem.
+  // If a symbol was created because of its appearance in a declare_target,
+  // a use in a call should override it with the procedure symbol.
+  Scope *ompDTScope{nullptr};
+  // A name implicitly declared by a DECLARE_TARGET may have been followed
+  // by an explcit declaration. Make sure the symbol is still implicit
+  // before doing anything.
+  if (WasDeclaredByOmpDeclareTarget(symbol) &&
+      symbol->flags().test(Symbol::Flag::Implicit)) {
+    // Implicit declaration of a symbol caused by being on a declare_target
+    // should only declare it as an object, not a procedure. This is because
+    // the 'x' in declare_target(x) looks like a use of a variable, not a
+    // procedure.
+    assert(!IsProcedure(*symbol) && "Should not be a procedure");
+    ompDTScope = const_cast<Scope *>(&symbol->owner());
+    ompDTScope->erase(symbol->name());
----------------
MattPD wrote:

`Scope::erase` removes the name's entry from the scope's symbol map, but the same `Symbol*` may already sit in an `EquivalenceObject` set or a `CommonBlockDetails` member list. Those references are not withdrawn, so a `Function` ends up sitting in an equivalence set or common block. The program then compiles without error and is lowered to inconsistent HLFIR (a storage `alloca` for the object alongside a `fir.call @_QPbaz`). Would it be worth checking the object is not referenced by an equivalence set or common block before erasing and recreating it as a procedure?

Current trunk (before this PR) rejects this case: https://flang.godbolt.org/z/anKccEx71

A smaller related case in the same recovery path: a name used as a variable and then called in the same scope (`a = baz` then `a = baz(a)`) is rejected at baseline (https://flang.godbolt.org/z/rafdq93Yo) but accepted here.

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