[Flang][OpenMP][Sema] More gracefully handle undefined symbol in a no implicit module for declare target

[agozillon]

Summary

Prior to this change, if you define a module as such with a declare target in it:

module test_0
    implicit none
!$omp declare target(no_implicit_materialization_1)
end module test_0

The compiler will crash rather than give some form of reasonable
diagnostic. This patch attempts to fix that.

From what I understand of Fortran, if I remove the implicit none
from the example module, the above code perhaps becomes
legal, as Fortran seems happy to materialize a type for symbols to
make them a variable. Although, I'm not sure what Fortrans
interactions are with implicitly defining variables inside of a
pragma/directive. At the very least Flang does this happily
at the semantic level for the moment.

[kiranchandramohan]

LG.

[agozillon]

Thank you @kiranchandramohan for the quick review!

flang/lib/Semantics/check-omp-structure.h

private:
   void CheckDependList(const parser::DataRef &);
   void CheckDependArraySection(
       const common::Indirection<parser::ArrayElement> &, const parser::Name &);
   bool IsDataRefTypeParamInquiry(const parser::DataRef *dataRef);
   void CheckIsVarPartOfAnotherVar(
       const parser::CharBlock &source, const parser::OmpObjectList &objList);
   void CheckThreadprivateOrDeclareTargetVar(
       const parser::OmpObjectList &objList);
+  void CheckSymbolNames(
+      const parser::CharBlock &source, const parser::OmpObjectList &objList);
   void CheckIntentInPointer(
       const parser::OmpObjectList &, const llvm::omp::Clause);
   void GetSymbolsInObjectList(const parser::OmpObjectList &, SymbolSourceMap &);
   void CheckDefinableObjects(SymbolSourceMap &, const llvm::omp::Clause);
   void CheckCopyingPolymorphicAllocatable(
       SymbolSourceMap &, const llvm::omp::Clause);
   void CheckPrivateSymbolsInOuterCxt(
       SymbolSourceMap &, DirectivesClauseTriple &, const llvm::omp::Clause);

flang/lib/Semantics/check-omp-structure.cpp

 
 void OmpStructureChecker::CheckThreadprivateOrDeclareTargetVar(
     const parser::OmpObjectList &objList) {
   for (const auto &ompObject : objList.v) {
     common::visit(
         common::visitors{
             [&](const parser::Designator &) {
               if (const auto *name{parser::Unwrap<parser::Name>(ompObject)}) {
+                // The symbol is null, return early, CheckSymbolNames
+                // should have already reported the missing symbol as a
+                // diagnostic error
+                if (!name->symbol) {
+                  return;
+                }
+
                 if (name->symbol->GetUltimate().IsSubprogram()) {
                   if (GetContext().directive ==
                       llvm::omp::Directive::OMPD_threadprivate)
                     context_.Say(name->source,
                         "The procedure name cannot be in a %s "
                         "directive"_err_en_US,
                         ContextDirectiveAsFortran());
                   // TODO: Check for procedure name in declare target directive.
void OmpStructureChecker::Enter(const parser::OpenMPThreadprivate &c) {
   const auto &dir{std::get<parser::Verbatim>(c.t)};
   PushContextAndClauseSets(
       dir.source, llvm::omp::Directive::OMPD_threadprivate);
 }
 
 void OmpStructureChecker::Leave(const parser::OpenMPThreadprivate &c) {
   const auto &dir{std::get<parser::Verbatim>(c.t)};
   const auto &objectList{std::get<parser::OmpObjectList>(c.t)};
+  CheckSymbolNames(dir.source, objectList);
   CheckIsVarPartOfAnotherVar(dir.source, objectList);
   CheckThreadprivateOrDeclareTargetVar(objectList);
   dirContext_.pop_back();
 }
 
 void OmpStructureChecker::Enter(const parser::OpenMPDeclareSimdConstruct &x) {
   const auto &dir{std::get<parser::Verbatim>(x.t)};
   PushContextAndClauseSets(dir.source, llvm::omp::Directive::OMPD_declare_simd);
void OmpStructureChecker::Enter(const parser::OpenMPDeclareTargetConstruct &x) {
   const auto &dir{std::get<parser::Verbatim>(x.t)};
   PushContext(dir.source, llvm::omp::Directive::OMPD_declare_target);
   const auto &spec{std::get<parser::OmpDeclareTargetSpecifier>(x.t)};
   if (std::holds_alternative<parser::OmpDeclareTargetWithClause>(spec.u)) {
     SetClauseSets(llvm::omp::Directive::OMPD_declare_target);
   }
 }
 
+void OmpStructureChecker::CheckSymbolNames(
+    const parser::CharBlock &source, const parser::OmpObjectList &objList) {
+  for (const auto &ompObject : objList.v) {
+    common::visit(
+        common::visitors{
+            [&](const parser::Designator &designator) {
+              if (const auto *name{parser::Unwrap<parser::Name>(ompObject)}) {
+                if (!name->symbol) {
+                  context_.Say(source,
+                      "The given %s directive clause has an invalid argument"_err_en_US,
+                      ContextDirectiveAsFortran());
+                }
+              }
+            },
+            [&](const parser::Name &name) {
+              if (!name.symbol) {
+                context_.Say(source,
+                    "The given %s directive clause has an invalid argument"_err_en_US,
+                    ContextDirectiveAsFortran());
+              }
+            },
+        },
+        ompObject.u);
+  }
+}
+
 void OmpStructureChecker::Leave(const parser::OpenMPDeclareTargetConstruct &x) {
   const auto &dir{std::get<parser::Verbatim>(x.t)};
   const auto &spec{std::get<parser::OmpDeclareTargetSpecifier>(x.t)};
   if (const auto *objectList{parser::Unwrap<parser::OmpObjectList>(spec.u)}) {
+    CheckSymbolNames(dir.source, *objectList);
     CheckIsVarPartOfAnotherVar(dir.source, *objectList);
     CheckThreadprivateOrDeclareTargetVar(*objectList);
   } else if (const auto *clauseList{
                  parser::Unwrap<parser::OmpClauseList>(spec.u)}) {
     for (const auto &clause : clauseList->v) {
       if (const auto *toClause{std::get_if<parser::OmpClause::To>(&clause.u)}) {
+        CheckSymbolNames(dir.source, toClause->v);
         CheckIsVarPartOfAnotherVar(dir.source, toClause->v);
         CheckThreadprivateOrDeclareTargetVar(toClause->v);
       } else if (const auto *linkClause{
                      std::get_if<parser::OmpClause::Link>(&clause.u)}) {
+        CheckSymbolNames(dir.source, linkClause->v);
         CheckIsVarPartOfAnotherVar(dir.source, linkClause->v);
         CheckThreadprivateOrDeclareTargetVar(linkClause->v);
       }
     }
   }
   dirContext_.pop_back();
 }
 
for (auto it : GetContext().clauseInfo) {
     const auto *clause = it.second;
     if (!threadprivateAllowedSet.test(type)) {
       if (const auto *objList{GetOmpObjectList(*clause)}) {
         for (const auto &ompObject : objList->v) {
           common::visit(
               common::visitors{
                   [&](const parser::Designator &) {
                     if (const auto *name{
-                            parser::Unwrap<parser::Name>(ompObject)})
+                            parser::Unwrap<parser::Name>(ompObject)}) {
+                      if (name->symbol) {
                         testThreadprivateVarErr(
                             name->symbol->GetUltimate(), *name, type);
+                      }
+                    }
                   },
                   [&](const parser::Name &name) {
                     if (name.symbol) {
                       for (const auto &mem :
                           name.symbol->get<CommonBlockDetails>().objects()) {
                         testThreadprivateVarErr(mem->GetUltimate(), name, type);
                         break;
                       }

flang/test/Semantics/OpenMP/declare-target06.f90

+! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp
+! OpenMP Version 5.1
+! Check OpenMP construct validity for the following directives:
+! 2.14.7 Declare Target Directive
+! When used in an implicit none context.
+
+module test_0
+    implicit none
+!ERROR: The given DECLARE TARGET directive clause has an invalid argument
+!ERROR: No explicit type declared for 'no_implicit_materialization_1'
+!$omp declare target(no_implicit_materialization_1)
+
+!ERROR: The given DECLARE TARGET directive clause has an invalid argument
+!ERROR: No explicit type declared for 'no_implicit_materialization_2'
+!$omp declare target link(no_implicit_materialization_2)
+
+!ERROR: The given DECLARE TARGET directive clause has an invalid argument
+!ERROR: No explicit type declared for 'no_implicit_materialization_3'
+!$omp declare target to(no_implicit_materialization_3)
+
+INTEGER :: data_int = 10
+!$omp declare target(data_int)
+end module test_0