[PATCH] D86322: [flang] Fix bug accessing implicit variable in specification expression

[Peter Klausler via Phabricator via llvm-commits]

klausler accepted this revision.
klausler added inline comments.


================
Comment at: flang/include/flang/Semantics/symbol.h:486
       Implicit, // symbol is implicitly typed
+      ImplicitOrError, // symbol must be implicitly defined or it's an error
       ModFile, // symbol came from .mod file
----------------
"implicitly typed"


================
Comment at: flang/lib/Semantics/check-declarations.cpp:770
+    if (details.implicitOrSpecExprError) {
+      messages_.Say("Implicitly declared local '%s' not allowed in"
+                    " specification expression"_err_en_US,
----------------
"implicitly typed" might be better here too.


================
Comment at: flang/lib/Semantics/resolve-names.cpp:1937
+template <typename T> static T &GetInclusiveScope(T &scope) {
+  for (T *s{&scope};; s = &s->parent()) {
+    if (s->kind() != Scope::Kind::Block && !s->IsDerivedType() &&
----------------
Consider adding a "while" predicate to this loop.  It'll never be false, but it would remove the need for the reader to have to think about it.


================
Comment at: flang/lib/Semantics/resolve-names.cpp:5486
+// be wrong we report an error later in CheckDeclarations().
+bool DeclarationVisitor::CheckForHostAssociatedImplicit(
+    const parser::Name &name) {
----------------
Can some of this logic be simplified if the internal procedure or the host has `IMPLICIT NONE`?  Or maybe I'm missing out on how it works.  But it seems like we need these new features only when neither scope has `IMPLICIT NONE`.


================
Comment at: flang/lib/Semantics/resolve-names.cpp:6007
+                    std::get<std::list<parser::CommonBlockObject>>(block.t)) {
+                  DeclareObjectEntity(std::get<parser::Name>(object.t))
+                      .set(Symbol::Flag::InCommonBlock);
----------------
Instead of using a new flag here, you could declare the common block's symbol and point the block's objects to it.


================
Comment at: flang/test/Semantics/block-data01.f90:10
   integer :: uninitialized ! ok
-  !ERROR: 'p' may not appear in a BLOCK DATA subprogram
+  !ERROR: 'p' may not be a procedure as it is in a COMMON block
   procedure(sin), pointer :: p => cos
----------------
Is the old error message now dead?  Maybe there's useless code that can be cleaned out from check-declarations.cpp.


================
Comment at: flang/test/Semantics/implicit11.f90:52
+end
+
----------------
Consider adding a test case with `IMPLICIT NONE` in the host procedure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86322