Page MenuHomePhabricator

[flang] Handle undeclared names in EQUIVALENCE statements

Authored by PeteSteinfeld on Dec 15 2020, 2:53 PM.



Names in EQUIVALENCE statements are only allowed to indicate local
objects as per, paragraph 2, item (10). Thus, a name appearing
in an EQUIVALENCE statement with no corresponding declaration in the
same scope is an implicit declaration of the name. If that scope
contains an IMPLICIT NONE, it's an error.

I implemented this by adding a state variable to the SemanticsContext to
indicate if we're resolving the names in an EQUIVALENCE statement and
then checked this state when resolving names. I also added a test to
the existing tests for EQUIVALENCE statements.

Diff Detail

Unit TestsFailed

15,710 msx64 debian > LLVM.Bindings/Go::go.test
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/bin/llvm-go go=/usr/bin/go test
50 msx64 windows > LLVM.CodeGen/XCore::threads.ll
Script: -- : 'RUN: at line 1'; c:\ws\w32-1\llvm-project\premerge-checks\build\bin\llc.exe -march=xcore < C:\ws\w32-1\llvm-project\premerge-checks\llvm\test\CodeGen\XCore\threads.ll | c:\ws\w32-1\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w32-1\llvm-project\premerge-checks\llvm\test\CodeGen\XCore\threads.ll

Event Timeline

PeteSteinfeld requested review of this revision.Dec 15 2020, 2:53 PM
PeteSteinfeld created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptDec 15 2020, 2:53 PM
PeteSteinfeld added a project: Restricted Project.Dec 15 2020, 2:53 PM
klausler requested changes to this revision.Dec 15 2020, 3:58 PM
klausler added inline comments.
133 ↗(On Diff #312045)

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.)

88 ↗(On Diff #312045)

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.

This revision now requires changes to proceed.Dec 15 2020, 3:58 PM
tskeith added inline comments.Dec 15 2020, 4:22 PM

FindInScope(scope, name.source) does the same thing, so you don't need the new function. (Or replace FindInScope with FindLocalSymbol everywhere.)


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.

Sorry, I didn't see Peter's comments until after I submitted mine.

PeteSteinfeld added inline comments.Dec 16 2020, 10:42 AM
133 ↗(On Diff #312045)

Thanks, Peter. I plan to follow Tim's suggestion below and use the existing

88 ↗(On Diff #312045)

Thanks. I'll make that change.


I didn't notice

```.  I'll just use that.

Thanks, Tim. Will do.

Responses to review comments. I moved the state variable indicating if we're
resolving names in an EQUIVALENCE statement to the class ScopeHandler and used
the existing FindInScope() function to limit name loopups to the current scope
rather than inventing a new interface.

klausler accepted this revision.Dec 16 2020, 10:48 AM
This revision is now accepted and ready to land.Dec 16 2020, 10:48 AM
tskeith accepted this revision.Dec 16 2020, 11:01 AM
This revision was landed with ongoing or failed builds.Dec 16 2020, 11:04 AM
This revision was automatically updated to reflect the committed changes.