[PATCH] D88279: SAVE statement should not apply to nested scoping units

[Tim Keith via Phabricator via llvm-commits]

tskeith accepted this revision.
tskeith added inline comments.
This revision is now accepted and ready to land.


================
Comment at: flang/lib/Evaluate/tools.cpp:1008
+    } else if (scope->hasSAVE() ) {
+      return true;
     }
----------------
rikigigi wrote:
> klausler wrote:
> > tskeith wrote:
> > > It's not clear to me why the call to `hasSAVE` is necessary at all. In resolve-names.cpp we set the SAVE attribute on the appropriate entities when a SAVE statement is encountered. So the symbol should have the SAVE attribute set here if SAVE is set in the scope.
> > > 
> > > I think you can remove the call to `hasSAVE` completely. And since it's the only call, remove the function and related stuff.
> > > 
> > > @klausler, do you know of any reason we need `hasSAVE`?
> > I recall that a "bare" `SAVE` statement in a scope wasn't causing name resolution to set the attribute on all items in the scope because it wasn't clear whether each entity was an "allowed item" in the sense of 8.6.14 and C859 -- a bare `SAVE` can appear before the other declarations that make it clear whether an entity is a common block, variable, or procedure pointer, and so the decision about SAVE had to be deferred.
> So do you think that I should remove completely the hasSAVE call or it is better to keep it?
We seem to have two methods of handling a bare SAVE statement, in resolve-names.cpp and here. That should be cleaned up so there is only one. It would be nice to have a test that shows where the resolve-names.cpp mechanism doesn't work.

But none of that has to happen to get this bug fixed, so I think it's fine to leave it as it is.


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

https://reviews.llvm.org/D88279