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.

Sign up for GitHub

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

Jump to bottom

Backport: nested function name canonicalization #179

Open
wants to merge 3 commits into
base: v1.10.2+RAI
Choose a base branch
from
Open

Backport: nested function name canonicalization #179

wants to merge 3 commits into from

Conversation

kpamnany
Copy link
Collaborator

@kpamnany kpamnany commented Sep 11, 2024 •
edited
Loading

PR Description

Backports JuliaLang#53719.

Checklist

Requirements for merging:

@github-actions github-actions bot added port-to-v1.10 This change should apply to Julia v1.10 builds port-to-master This change should apply to all future Julia builds labels Sep 11, 2024
d-netto
d-netto reviewed Sep 11, 2024
View reviewed changes
Copy link
Member

@d-netto d-netto left a comment

Choose a reason for hiding this comment

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

Have a minor comment.

I'd be more confident on this backport if you could run the JuliaLang tests locally on your machine and report the results here.

src/julia-syntax.scm Outdated Show resolved Hide resolved
@kpamnany
Copy link
Collaborator Author

Some failures:

<snip>
asyncmap                                         (15) |    17.35 |   0.38 |  2.2 |     682.96 |   579.92                                                                               
SparseArrays/linalg_solvers                      (15) |        started at 2024-09-12T14:43:22.234                                                                                      
show                                             (31) |         failed at 2024-09-12T14:43:22.912                                                                                      
Test Failed at /home/ubuntu/julia.rai/test/show.jl:790                                                                                                                                 
  Expression: occursin(r"var\"#foo2##[0-9]+", sprint(show, (M53719.foo2(1))(2), context = :module => M53719))                                                                          
   Evaluated: occursin(r"var\"#foo2##[0-9]+", "var\"#bar2##1#bar2##2\"{Int64, Int64}(2, 1)")                                                                                           
                                                                                                                                                                                       
Test Failed at /home/ubuntu/julia.rai/test/show.jl:801                                                                                                                                 
  Expression: occursin(r"var\"#bar#foo##[0-9]+", sprint(show, typeof(M53719.foo(1)), context = :module => M53719))                                                                     
   Evaluated: occursin(r"var\"#bar#foo##[0-9]+", "var\"#bar#bar##0\"{Int64}")                                                                                                          
                                                                                                                                                                                       
Test Failed at /home/ubuntu/julia.rai/test/show.jl:803                                                                                                                                 
  Expression: occursin(r"var\"#baz#foo##[0-9]+", sprint(show, typeof((M53719.foo(1))(2)), context = :module => M53719))                                                                
   Evaluated: occursin(r"var\"#baz#foo##[0-9]+", "var\"#baz#bar##1\"{Int64, Int64}")                                                                                                   
                                                                                                                                                                                       
Test Failed at /home/ubuntu/julia.rai/test/show.jl:805                                                                                                                                 
  Expression: occursin(r"var\"#bar2#foo2##[0-9]+", sprint(show, typeof(M53719.foo2(1)), context = :module => M53719))                                                                  
   Evaluated: occursin(r"var\"#bar2#foo2##[0-9]+", "var\"#bar2#bar2##0\"{Int64}")                                                                                                      
                                                                                                                                                                                       
Test Failed at /home/ubuntu/julia.rai/test/show.jl:807                                                                                                                                 
  Expression: occursin(r"var\"#foo2##[0-9]+#foo2##[0-9]+", sprint(show, typeof((M53719.foo2(1))(2)), context = :module => M53719))                                                     
   Evaluated: occursin(r"var\"#foo2##[0-9]+#foo2##[0-9]+", "var\"#bar2##1#bar2##2\"{Int64, Int64}")
<snip>

@kpamnany kpamnany added port-to-v1.10 This change should apply to Julia v1.10 builds and removed port-to-v1.10 This change should apply to Julia v1.10 builds port-to-master This change should apply to all future Julia builds labels Sep 15, 2024
d-netto and others added 3 commits September 20, 2024 20:19
@d-netto
Canonicalize names of nested functions by keeping a more fine grained…
3eb8a3f 
… counter -- per (module, method name) pair (JuliaLang#53719)

As mentioned in JuliaLang#53716, we've
been noticing that `precompile` statements lists from one version of our
codebase often don't apply cleanly in a slightly different version.

That's because a lot of nested and anonymous function names have a
global numeric suffix which is incremented every time a new name is
generated, and these numeric suffixes are not very stable across
codebase changes.

To solve this, this PR makes the numeric suffixes a bit more fine
grained: every pair of (module, top-level/outermost function name) will
have its own counter, which should make nested function names a bit more
stable across different versions.

This PR applies @JeffBezanson's idea of making the symbol name changes
directly in `current-julia-module-counter`.

Here is an example:

```Julia
julia> function foo(x)
           function bar(y)
               return x + y
           end
       end
foo (generic function with 1 method)

julia> f = foo(42)
(::var"#bar#foo##0"{Int64}) (generic function with 1 method)
```
@kpamnany @d-netto
Address review comment
ce4606c
@d-netto
fix is-method?
df219a8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@d-netto d-netto d-netto left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
port-to-v1.10 This change should apply to Julia v1.10 builds
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kpamnany @d-netto