[PATCH] D82903: [flang] Bug fix for ambiguous references to data and functions

[Pete Steinfeld via Phabricator via llvm-commits]

PeteSteinfeld marked 2 inline comments as done.
PeteSteinfeld added inline comments.


================
Comment at: flang/lib/Semantics/resolve-names.cpp:5885
+    if (!symbol) {
+      symbol = currScope().FindSymbol(name->source);
+    }
----------------
tskeith wrote:
> PeteSteinfeld wrote:
> > tskeith wrote:
> > > Why is this change needed? When would `FindSymbol` find the wrong symbol?
> > Yes.  The call to `currScope()` misses block scopes.  So if the same name is declared in a block and in the global scope, the wrong one will be found.
> So you can't call `FindSymbol` if the name is in a block scope. But then how do you know it won't be called in that case? Is there a guarantee that every name in a block scope will be resolved by this point and so `FindSymbol` won't be called on it?
I'm not sure I understand your question.  This code gets executed twice, once during processing of the specification part and then again at the end of processing of the execution part.  In either case, we won't find names declared in block scopes.  During the second pass, we won't find names that are declared in block scopes that have not had their symbol filled in previously.  I'm not sure if this latter case is possible for an error-free symbol.


================
Comment at: flang/lib/Semantics/resolve-names.cpp:7030
+  // not have symbols
+  ExecutionPartSkimmer{*this}.Walk(node.exec());
 }
----------------
tskeith wrote:
> PeteSteinfeld wrote:
> > tskeith wrote:
> > > Why does this require an extra pass over the parse tree. Can't this check be done as part of `ResolveExecutionParts` where `ArrayElement` nodes are handled?
> > The problem I ran into was dealing with blocks and their associated scopes.  When processing the specification parts, I couldn't find symbols declared in blocks.  I tried eliminating the pass in `ResolveSpecificationParts`, but that broke processing of subprograms as dummy arguments.
> I was referring to this call, not the one in `ResolveSpecificationParts`. Immediately above here we walk the execution parts. The will include the `ArrayElement` node. Why can't `NoteExecutablePartArrayElement` be done as part of that walk?
Thanks for clarifying.  It may be possible to do that.  I'll investigate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82903