[PATCH] D93345: [flang] Handle undeclared names in EQUIVALENCE statements

[Pete Steinfeld via Phabricator via llvm-commits]

PeteSteinfeld added inline comments.


================
Comment at: flang/include/flang/Semantics/scope.h:133
 
+  // Look for symbol by name only in this scope (for EQUIVALENCE statements)
+  Symbol *FindLocalSymbol(const SourceName &) const;
----------------
klausler wrote:
> This is really just a convenience wrapper for `find()`, yes?  If so, then either it's redundant with `find()`, or you should find and replace other uses of `find()` with your new API and avoid the confusion that can arise with multiple interfaces that do much the same thing.  (Preference is for the former, since usage of `find()` in scopes is pretty idiomatic by this point.)
Thanks, Peter.  I plan to follow Tim's suggestion below and use the existing 
```
FindInScope()
```


================
Comment at: flang/include/flang/Semantics/semantics.h:88
   bool debugModuleWriter() const { return debugModuleWriter_; }
+  bool inEquivalenceStmt() { return inEquivalenceStmt_; }
   const evaluate::IntrinsicProcTable &intrinsics() const { return intrinsics_; }
----------------
klausler wrote:
> The SemanticsContext object is probably not the best place for this new state.  One of the visitor classes in name resolution would make more sense.
Thanks.  I'll make that change.


================
Comment at: flang/lib/Semantics/resolve-names.cpp:2024
+    return Resolve(name,
+        context().inEquivalenceStmt() ? scope.FindLocalSymbol(name.source)
+                                      : scope.FindSymbol(name.source));
----------------
tskeith wrote:
> `FindInScope(scope, name.source)` does the same thing, so you don't need the new function. (Or replace FindInScope with FindLocalSymbol everywhere.)
I didn't notice 
```
FindInScope()
```.  I'll just use that.


================
Comment at: flang/lib/Semantics/resolve-names.cpp:4358
   EquivalenceSets equivSets{context()};
+  context().set_inEquivalenceStmt(true);
   for (const auto *set : equivalenceSets_) {
----------------
tskeith wrote:
> I think it makes more sense for `inEquivalenceStmt_` to be recorded in `ScopeHandler` (like `inExecutionPart_`, for example). It doesn't really belong in SemanticsContext because it only applies to name resolution, not to the rest of semantic analysis.
Thanks, Tim.  Will do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93345