[PATCH] D92672: [flang][openacc] Enforce restriction on routine directive and clauses

[Valentin Clement via Phabricator via llvm-commits]

clementval marked an inline comment as done.
clementval added inline comments.


================
Comment at: flang/include/flang/Parser/parse-tree.h:3823
+  UNION_CLASS_BOILERPLATE(AccBindClause);
+  std::variant<Name, ScalarDefaultCharExpr> u;
+  CharBlock source;
----------------
sameeranjoshi wrote:
> Sorry I couldn't find where is this defined in standard.
> Could you please point that?
3.1 line 2732 and 2733


================
Comment at: flang/lib/Semantics/check-acc-structure.cpp:221
+    const Scope &containingScope{GetProgramUnitContaining(scope)};
+    if (containingScope.kind() == Scope::Kind::Module) {
+      context_.Say(GetContext().directiveSource,
----------------
sameeranjoshi wrote:
> Where is the check for `inside subroutine/function as well as inside interface` done?
> Sorry I might be missing something, but I see you check only for the module scope of the construct.
The only place where you cannot have routine directive without a name is in the module specification part. So the test here is kind of inverted if you want. Instead of checking where it's allowed, I'm checking where it is not. 


================
Comment at: flang/test/Semantics/acc-routine-validity.f90:19
+
+  !ERROR: Internal: no symbol found for 'dummy'
+  !$acc routine(dummy) seq
----------------
sameeranjoshi wrote:
> Why isn't it getting resolved though having a resolving function defined above?
> Do you think there is a need for a more better resolver, which I tried in 
> https://reviews.llvm.org/D93051 with the latest diff?
`dummy` is not defined anywhere and must raise an error here. There is no subroutine/function named dummy so no symbol so the error is triggered. This is the correct behavior. Not all name can be resolved if they really do not exists. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92672