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

[Tim Keith via Phabricator via llvm-commits]

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


================
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) {
----------------
klausler wrote:
> 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`.
If the internal procedure has `IMPLICIT NONE` but the host does not, we still need this. That's why we don't get an error on `m` in subroutine `s1a` in implicit11.f90.

I've changed this function to exit early when the host 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);
----------------
klausler wrote:
> Instead of using a new flag here, you could declare the common block's symbol and point the block's objects to it.
Yes, that's a better way to do it. Now it's all done in CreateCommonBlockSymbols and that allows the main visitor to be simplified.


================
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
----------------
klausler wrote:
> Is the old error message now dead?  Maybe there's useless code that can be cleaned out from check-declarations.cpp.
It can still happen. I've added a new test that produces it.


================
Comment at: flang/test/Semantics/implicit11.f90:52
+end
+
----------------
klausler wrote:
> Consider adding a test case with `IMPLICIT NONE` in the host procedure.
Thanks -- I did that and it turned up another small change that was needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86322