Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TransformInline: Fix rescoping in expression substitution #170

Merged
merged 2 commits into from
Oct 12, 2023

Conversation

mlange05
Copy link
Collaborator

A small fix the the member inlining utility and accompanying test.

The problem was the wrong use of symbol substitution, which we now face to rescope and rebuild when replicating the body of the child routine to the parent. This would otherwise in-place update associate statements, as they are type definitions, which creates issues for replicated calls, as the same object is used for separate substitutions.

Many thanks to @rolfhm for finding, flagging and triaging the problem and providing the accompanying test case.

This would otherwise in-place update associate statements, as they
are type definitions. This would create issues for replicated calls
when symbols are not rescoped properly.
@github-actions
Copy link

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

@mlange05 mlange05 requested a review from reuterbal October 12, 2023 14:19
@codecov
Copy link

codecov bot commented Oct 12, 2023

Codecov Report

Merging #170 (0de4f6f) into main (ede5135) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #170      +/-   ##
==========================================
+ Coverage   92.12%   92.13%   +0.01%     
==========================================
  Files          90       90              
  Lines       16689    16689              
==========================================
+ Hits        15374    15376       +2     
+ Misses       1315     1313       -2     
Flag Coverage Δ
lint_rules 96.22% <ø> (ø)
loki 92.10% <100.00%> (+0.01%) ⬆️
transformations 91.35% <ø> (ø)

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

Files Coverage Δ
loki/transform/transform_inline.py 87.21% <100.00%> (ø)

... and 1 file with indirect coverage changes

Copy link
Collaborator

@reuterbal reuterbal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, nice find and fix. This is really subtle... GTG

@reuterbal reuterbal merged commit 03324ef into main Oct 12, 2023
@reuterbal reuterbal deleted the naml-member-inliner-associate branch October 12, 2023 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants