Types: New Scope accessors, refactoring and LazyNodeLookup removal by mlange05 · Pull Request #647 · ecmwf-ifs/loki

mlange05 · 2026-01-27T19:00:34Z

~~Note: This sits on top of PR #646 and should only be reviewed after that is merged.~~

The primary purpose of this PR is introduce a new "Scope accessor" API and use it to avoid repeated cloning and re-instantiating of symbols for the sole purpose of updating symbol/type attributes. The key idea is to make insertions and updates into the symbol-table explicitly by providing the methods s.declare(name, **attrs) / s.update(name, **attrs), as well as s.get_type(name) and s.get_dtype(name) to Scope objects. This should allow us to gradually only adjust type information through this high-level API, and optionally fail if there is an unexpected state - updating an undeclared symbol or re-declaring something that's already been declared.

In the long-run this is intended to replace all manual modifications to scope.symbol_attrs, but in this PR I've addressed two of the main inconsistencies in the FParser frontend:

When declaring variables, we now let the Entity_Decl (the first time we have the variable name) declare the base type (LHS of the ::), before creating the corresponding symbol. We then recurse on the symbol itself to pick up RHS attributes (implicit shape/dimension or initial value) and update the declaration accordingly.
For StatementFunction objects, we no longer use the LazyNodeLookup, but instead explicitly "re-declare" the base type to change from "array" to procedure type.

The latter also required a more low-level intervention: Making StatementFunction nodes ScopedNode, so that they no longer get cloned on default Transformer passes. A bunch of tests has also been added to ensure this behaviour more explicitly.

Other small additions and cosmetids:

Expose the enum BasicType values directly, so that we can do from loki.types import DEFERRED
Move tests for contained/internal procedures into a separate test and chance nomenclature from "members" to "internal"
Remove now redundant LazyNodeLookup entirely

github-actions · 2026-01-27T19:03:58Z

Documentation for this branch can be viewed at https://sites.ecmwf.int/docs/loki/647/index.html

codecov · 2026-01-29T07:07:52Z

Codecov Report

❌ Patch coverage is 99.17355% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.41%. Comparing base (b711b95) to head (9e029b0).

Files with missing lines	Patch %	Lines
loki/analyse/analyse_dataflow.py	33.33%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #647      +/-   ##
==========================================
+ Coverage   96.39%   96.41%   +0.02%     
==========================================
  Files         266      267       +1     
  Lines       46418    46519     +101     
==========================================
+ Hits        44745    44852     +107     
+ Misses       1673     1667       -6

Flag	Coverage Δ
lint_rules	`96.40% <ø> (ø)`
loki	`96.41% <99.17%> (+0.02%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Instead of pre-creating dummy variables and then updating them once the types are known, we now pass the type on, declare the base type we know (lLHS data types), and then update the `initial` or implicit `dimension` when we see it in parsing the RHS of the declaration.

Before parsing the spec of a `Function` with implicit return type, we need to dummy-declare it locally, so that the in-place type declaration does not pick up the `ProcedureType` that the parent sees.

By default `Transformer` passes will re-generate IR nodes. Statement functions, however, need to be maintained, as they are the target of `ProcedureTypes`. This avoids the weakref dropping its target, and subsequent passes misidentifying symbols.

When inlining nested stmt functions we need to force a rescoping, as intrinsic functions might now have been rescoped correctly. This is because the recurisve map-update does not force the rescoping of via the `InlineSubstitutionMapper`.

reuterbal

Many thanks, a very nice and important step towards the overhaul of the symbol type system! Only some remarks about minor details, no objections per se!

reuterbal · 2026-03-16T15:24:37Z

loki/analyse/analyse_dataflow.py

+    def visit_StatementFunction(self, o, **kwargs):
+        # Only the return symbol is meaningful to the outside
+        defines = self._symbols_from_expr(o.variable)
+        return self.visit_Node(o, defines_symbols=defines, **kwargs)


This seems not to be covered by tests - any chance of monkey-patching this into an existing test to make sure it behaves as expected?

reuterbal · 2026-03-19T12:28:19Z

loki/ir/nodes.py

+    Internally, this is considered a :any:`ScopedNode` with an empty
+    body, because it may be the target of a :any:`ProcedureType`.


Suggested change

Internally, this is considered a :any:`ScopedNode` with an empty

body, because it may be the target of a :any:`ProcedureType`.

Internally, this is considered a :any:`ScopedNode`, because

it may be the target of a :any:`ProcedureType`.

ScopedNode doesn't imply being an InternalNode, which is what would introduce a body - so I would leave that reference out.

reuterbal · 2026-03-19T12:31:52Z

loki/transformations/inline/functions.py


+    # Rescope the routine body, as the recursive update call does not
+    # set scopes on the RHS of `exprmap`, and so we might miss a scope.
+    routine.rescope_symbols()


Would it not be better to combine these two into a single pass, applying the rescoping only when substitution occurs?

reuterbal · 2026-03-19T12:35:18Z

loki/types/tests/test_procedure_types.py

+  implicit none
+  integer, intent(in) :: n
+  real(kind=4), intent(inout) :: a(3)
+  real(kind=4) :: pants, on, fire


reuterbal · 2026-03-19T15:07:47Z

loki/types/scope.py

        if self.parent is not None:
            self.symbol_attrs.parent = self.parent.symbol_attrs
+
+    def declare(self, name, dtype, fail=True, **kwargs):


Since this method (as well as update and the getters) constitute a rather crucial part of the new API, would you mind adding some meaningful docstrings to explain their role, use, and rationale?

Copilot

Pull request overview

This PR introduces a higher-level Scope accessor API (declare, update, get_type, get_dtype) and refactors type/scope handling to reduce symbol/type re-instantiation, including removing the LazyNodeLookup mechanism and tightening statement-function/procedure-type linkage.

Changes:

Add Scope declaration/update/getter helpers and corresponding unit tests.
Refactor FPARSER frontend handling for declarations and statement functions to use the new scope API and remove LazyNodeLookup.
Restructure/extend tests around procedure types, statement functions, and internal procedures; expose BasicType enum values as top-level imports.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
loki/types/scope.py	Adds `declare/update/get_type/get_dtype` APIs on `Scope`.
loki/types/tests/test_scope.py	Adds tests for the new `Scope` setter/getter APIs and updates BasicType usage.
loki/frontend/fparser.py	Refactors declaration/statement-function parsing to use new scope APIs; removes lazy lookup usage.
loki/ir/nodes.py	Makes `StatementFunction` a `ScopedNode` to preserve node identity through transformations.
loki/types/procedure_type.py	Removes `LazyNodeLookup` support and adds a `procedure` setter (weakref linkage).
loki/types/module_type.py	Removes `LazyNodeLookup` support and adds a `module` setter (weakref linkage).
loki/tools/util.py	Removes `LazyNodeLookup` implementation and export.
loki/types/datatypes.py	Exposes `BasicType` members as top-level constants (e.g., `INTEGER`, `REAL`).
loki/analyse/analyse_dataflow.py	Adds dataflow handling for `StatementFunction` nodes.
loki/transformations/inline/functions.py	Rescopes symbols after statement-function inlining substitutions.
loki/transformations/inline/tests/test_functions.py	Updates expectations to include intrinsic calls and scoping after inlining.
loki/types/tests/test_procedure_types.py	Adds a transform-stability test for statement-function procedure type links.
loki/program_unit.py	Adds `routines`/`procedures` aliases for `subroutines`.
loki/tests/test_subroutine.py	Moves internal-procedure coverage out to a dedicated test file.
loki/tests/test_internal_procedures.py	New test suite for internal/contained procedures, cloning, and aliasing behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

loki/frontend/fparser.py

        kwargs['scope'] = routine

+        # Define the return type in the local scope before parsing spec.
+        # If the return type is impliicit (function name), we need to


loki/types/procedure_type.py

+        self._is_function = is_function or False
+        self._return_type = return_type



loki/types/procedure_type.py

        assert isinstance(return_type, SymbolAttributes) or procedure or not is_function or is_intrinsic
        self.is_generic = is_generic
        self.is_intrinsic = is_intrinsic
-        if procedure is None or isinstance(procedure, LazyNodeLookup):
-            self._procedure = procedure
-            self._name = name
-            self._is_function = is_function or False
-            self._return_type = return_type
-            # NB: not applying an assert on the procedure name for LazyNodeLookup as
-            # the point of the lazy lookup is that we might not have the the procedure
-            # definition available at type instantiation time
-        else:
-            self._procedure = weakref.ref(procedure)
-            # Cache all properties for when procedure link becomes inactive
-            assert name is None or name.lower() == self.procedure.name.lower()
-            self._name = self.procedure.name
-            assert is_function is None or is_function == self.procedure.is_function
-            self._is_function = self.procedure.is_function
-            # TODO: compare return type once type comparison is more robust
-            self._return_type = self.procedure.return_type if self.procedure.is_function else None
+
+        self.procedure = procedure
+        self._name = procedure.name if procedure else name


loki/types/scope.py

+
+    def get_dtype(self, name, recursive=True, fail=True):
+
+        return self.get_type(name, recursive=recursive, fail=fail).dtype


loki/types/scope.py

+            raise ValueError(f'[Loki::Scope] Tyring to re-declare already declared symbol name: {name}')
+
+        self.symbol_attrs[name] = SymbolAttributes(dtype, **kwargs)
+
+    def update(self, name, fail=True, **kwargs):
+
+        # Ensure `dtype` defines a known type
+        if 'dtype' in kwargs:
+            assert isinstance(kwargs['dtype'], (DataType, str))
+
+        if fail and name not in self.symbol_attrs:
+            raise ValueError(f'[Loki::Scope] Tyring to update undeclared symbol name: {name}')


loki/frontend/fparser.py

+                scope.update(var.name, dtype=proc_type)

-            variables = tuple(var.rescope(scope=scope) for var in variables)
+            variables = tuple(var.clone(scope=scope) for var in variables)


loki/types/tests/test_procedure_types.py

+    # Check origianl stmt func is fine and linked
+    sftype = stmtfuncs[0].variable.type.dtype
+    assert isinstance(sftype, ProcedureType)
+    assert sftype.procedure == stmtfuncs[0]
+
+    assert isinstance(assigns[0].rhs, sym.InlineCall)
+    assert isinstance(assigns[0].rhs.function.type.dtype, ProcedureType)
+    assert assigns[0].rhs.function.type.dtype.procedure == stmtfuncs[0]
+
+    # Safe the original IR node and run a no-op Transformer over the spec
+    original = stmtfuncs[0]
+    routine.spec = Transformer().visit(routine.spec)


loki/types/module_type.py

        from loki.module import Module  # pylint: disable=import-outside-toplevel,cyclic-import
        super().__init__()
        assert name or isinstance(module, Module)
-        if module is None or isinstance(module, LazyNodeLookup):
-            self._module = module
-            self._name = name
-        else:
-            self._module = weakref.ref(module)
-            # Cache all properties for when module link becomes inactive
-            assert name is None or name.lower() == self.module.name.lower()
-            self._name = self.module.name
+
+        self.module = module
+        self._name = module.name if module else name


loki/tests/test_internal_procedures.py

+
+    b_outer = routine.get_type('b')
+    assert b_outer.dtype == INTEGER
+    assert b_outer.dtype == INTEGER


mlange05 requested a review from reuterbal January 27, 2026 19:00

mlange05 force-pushed the naml-scope-api-refactor branch 2 times, most recently from bb8b5b3 to a1bfe24 Compare January 29, 2026 06:44

mlange05 force-pushed the naml-scope-api-refactor branch from 0e91656 to 8a830d0 Compare February 7, 2026 19:18

mlange05 marked this pull request as ready for review February 8, 2026 04:42

mlange05 added 12 commits March 4, 2026 13:53

Types: Exposing the base data types directly

e7aad03

Types: Add declare/update/get_type/get_dtype methods to Scope

21b583b

Function: Ensure implicit return types derive the right local type

08ed154

Before parsing the spec of a `Function` with implicit return type, we need to dummy-declare it locally, so that the in-place type declaration does not pick up the `ProcedureType` that the parent sees.

Procedure: Move test for internal procedures to dedicated test

ca4f0be

Procedure: Change "members" to "internal procedures" and add alias

4d7ed3f

Procedure: Add test for variable aliasing in internal procedures

05bf2cc

FParser: Avoid using LazyNodeLookup forStatementFunctions

98c8fd9

Tools: Remove the now redundant LazyNodeLookup utility

655c6a3

Analysis: Treat new StatementFunction node in dataflow analysis

9e029b0

mlange05 force-pushed the naml-scope-api-refactor branch from 8a830d0 to 9e029b0 Compare March 4, 2026 12:53

reuterbal requested changes Mar 19, 2026

View reviewed changes

reuterbal requested a review from Copilot March 19, 2026 15:11

Copilot AI reviewed Mar 19, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Types: New Scope accessors, refactoring and LazyNodeLookup removal#647

Types: New Scope accessors, refactoring and LazyNodeLookup removal#647
mlange05 wants to merge 12 commits intomainfrom
naml-scope-api-refactor

mlange05 commented Jan 27, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Jan 27, 2026

Uh oh!

codecov bot commented Jan 29, 2026 •

edited

Loading

Uh oh!

reuterbal left a comment

Uh oh!

reuterbal Mar 16, 2026

Uh oh!

reuterbal Mar 19, 2026

Uh oh!

reuterbal Mar 19, 2026

Uh oh!

reuterbal Mar 19, 2026

Uh oh!

reuterbal Mar 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		Internally, this is considered a :any:`ScopedNode` with an empty
		body, because it may be the target of a :any:`ProcedureType`.

		self._is_function = is_function or False
		self._return_type = return_type


		def get_dtype(self, name, recursive=True, fail=True):

		return self.get_type(name, recursive=recursive, fail=fail).dtype

Conversation

mlange05 commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 27, 2026

Uh oh!

codecov bot commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

reuterbal left a comment

Choose a reason for hiding this comment

Uh oh!

reuterbal Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

reuterbal Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

reuterbal Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

reuterbal Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

reuterbal Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mlange05 commented Jan 27, 2026 •

edited

Loading

codecov bot commented Jan 29, 2026 •

edited

Loading