[flang] Insert NAMELIST variables into symbol table during Pre

[jotken]

Call ResolveName for each variable in a NAMELIST in Pre(). This generates errors for two requirements in section 8.9 of the fortran standard. Briefly, objects shall have their declared type specified by previous statements or implicit rules. If an object is typed by the implicit rules, any subsequent type statement shall confirm the implied type.

Update test resolve40 s5 so that subsequent type statement confirms namelist implied type.

Co-authored-by: John Otken john.otken@hpe.com

[llvmbot]

@llvm/pr-subscribers-flang-semantics

Author: John Otken (jotken)

Changes

Call ResolveName for each variable in a NAMELIST in Pre(). This generates errors for two requirements in section 8.9 of the fortran standard. Briefly, objects shall have their declared type specified by previous statements or implicit rules. If an object is typed by the implicit rules, any subsequent type statement shall confirm the implied type.

Update test resolve40 s5 so that subsequent type statement confirms namelist implied type.

Co-authored-by: John Otken john.otken@hpe.com

---

Full diff: https://github.com/llvm/llvm-project/pull/179599.diff

2 Files Affected:

• (modified) flang/lib/Semantics/resolve-names.cpp (+4)
• (modified) flang/test/Semantics/resolve40.f90 (+2-2)

diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index db84cc40c4283..efada9d6b5767 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -7195,6 +7195,10 @@ bool DeclarationVisitor::Pre(const parser::NamelistStmt::Group &x) {
     groupSymbol = &MakeSymbol(groupName, NamelistDetails{});
     groupSymbol->ReplaceName(groupName.source);
   }
+  // insert namelist variables into the symbol table
+  for (const auto &name : std::get<std::list<parser::Name>>(x.t)) {
+    ResolveName(name);
+  }
   // Name resolution of group items is deferred to FinishNamelists()
   // so that host association is handled correctly.
   GetDeferredDeclarationState(true)->namelistGroups.emplace_back(&x);
diff --git a/flang/test/Semantics/resolve40.f90 b/flang/test/Semantics/resolve40.f90
index 16b825250695e..466d704e9a46a 100644
--- a/flang/test/Semantics/resolve40.f90
+++ b/flang/test/Semantics/resolve40.f90
@@ -39,8 +39,8 @@ subroutine s4b
 end
 
 subroutine s5
-  namelist /nl/x
-  integer x
+  namelist /nl/i
+  integer i
 end
 
 subroutine s6

[jotken]

Paragraph 5 of 8.9 NAMELIST statement

A namelist group object shall either be accessed by use or host association or shall have its declared type, kind
type parameters of the declared type, and rank specified by previous statements in the same scoping unit or
by the implicit typing rules in effect for the scoping unit. If a namelist group object is typed by the implicit
typing rules, its appearance in any subsequent type declaration statement shall confirm the implied type and
type parameters.

[jotken]

This is my second (slightly larger) PR. I would appreciate your review and comments. @tskeith @klausler

I am open to limiting this change to -pedantic.

[eugeneepshteyn]

Note that in https://flang.llvm.org/docs/Extensions.html it says "Flang processes the NAMELIST group declarations in a scope after it has resolved all of the names in that scope. This means that names that appear before their local declarations do not resolve to host associated objects and do not elicit errors about improper redeclarations of implicitly typed entities."

So -pedantic warning is probably needed for this.

[eugeneepshteyn]

I am concerned about the comment on line 7202

[jotken]

Yes, FinishNamelists() runs late in the semantic phase so that NAMELIST i followed by REAL i types i to real instead of integer. I'm not sure of the motivation for this other than the comment about host association (and now the paragraph from Extensions you cited). I did test host association with this change and it works. I'm completely fine with -pendantic. I'm also okay with removing the comment. I tried to briefly describe what the code does and left out the why. This reflects a lot of time spent in resolve-names.cpp wondering what a block of code does. I'm easy so if there are no other comments, I'll make the -pendantic change. Do you want the comment deleted? Thanks again for your suggestions.

[klausler]

What problem does this patch solve? It looks like you're going out of your way to make it harder to port code to this compiler from the several others that defer NAMELIST processing. Don't do that. If you want to emit a pedantic warning about the use of this extension, fine, but this can't be an error by default.

[klausler]

Don't reduce portability to flang-new.

[eugeneepshteyn]

Don't change the existing test case with x (implicit real, but redefined as integer, with -pedantic warning), but add a new test case with i.

[ivanrodriguez3753]

Note that in https://flang.llvm.org/docs/Extensions.html it says "Flang processes the NAMELIST group declarations in a scope after it has resolved all of the names in that scope. This means that names that appear before their local declarations do not resolve to host associated objects and do not elicit errors about improper redeclarations of implicitly typed entities."

@eugeneepshteyn Could you provide some more context on why this decision was made? Or is it just the way things are? It seems like an intentional deviation from the standard, given its documented in the section "Behavior in cases where the standard is ambiguous or indefinite", which I would think we want to minimize, but I'm aware justifications and exceptions exist for everything. Furthermore, we can get the 'desired' behavior without violating the standard by using implicit none, right?

[klausler]

Note that in https://flang.llvm.org/docs/Extensions.html it says "Flang processes the NAMELIST group declarations in a scope after it has resolved all of the names in that scope. This means that names that appear before their local declarations do not resolve to host associated objects and do not elicit errors about improper redeclarations of implicitly typed entities."

@eugeneepshteyn Could you provide some more context on why this decision was made? Or is it just the way things are? It seems like an intentional deviation from the standard, given its documented in the section "Behavior in cases where the standard is ambiguous or indefinite", which I would think we want to minimize, but I'm aware justifications and exceptions exist for everything. Furthermore, we can get the 'desired' behavior without violating the standard by using implicit none, right?

It is indeed an intentional extension intended to ease porting of code from four other compilers that allow a name to appear in a NAMELIST prior to a type-declaration-stmt.

If IMPLICIT NONE fails to elicit an error with this extension, that should be fixed.

[ivanrodriguez3753]

I don't think the problem is a name appearing in a NAMELIST prior to a type-declaration-stmt, rather that the implicit type is not agreeing with the declaration, which is unambiguously stated in the spec quoted in @jotken's comment above.
Cray Fortran emits an error for this (correctly, I think) and has been for quite some time. If Cray Fortran is the only compiler that enforces this error, is there a request or discussion for rewording for the spec saying that items appearing in a namelist before declaration are NOT implicitly typed, or that their implicit types are overridden if a type declaration appears after it, or whatever other constraint/semantics that would make this work?
My interpretation was that i is implicitly typed the first time it appears, which is in the namelist, then the optional and present declaration for i is supposed to confirm the implicit type: "its appearance in any subsequent type declaration statement shall confirm the implied type and type parameters." It doesn't agree, hence Cray Fortran's error. flang and others seem to be okay with this. What I meant by using implicit none is that, with it, implicit typing rules aren't applied, and so the only typing of i is the one that is used, that is, the one in the declaration.

[klausler]

Please consult the first paragraph in flang/docs/Extensions.md, which lays out this compiler's policy on the support of language extensions. In short, if an extension exists in other compilers, is well-defined and portable, and doesn't invalidate a conforming program, we want to know about that extension and support it by default, because we want it to be as easy as possible to port existing code to this compiler. If you want this compiler to be strict in its enforcement of the ISO standard, you can make it so via the command line.