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

syntax: local variable name "N" conflicts with a static parameter #32816

Closed
tlienart opened this issue Aug 7, 2019 · 9 comments
Closed

syntax: local variable name "N" conflicts with a static parameter #32816

tlienart opened this issue Aug 7, 2019 · 9 comments
Assignees
@JeffBezanson
Labels
bug Indicates an unexpected problem or unintended behavior compiler:lowering Syntax lowering (compiler front end, 2nd stage)
Milestone
1.3

Comments

@tlienart
Copy link
Contributor

tlienart commented Aug 7, 2019 •
edited
Loading

Version: Julia 1.3/alpha // MacOS

This may be related to #29429 and was observed here: JuliaAI/MLJ.jl#198

I couldn't write a barebone reproducing example so unfortunately I can only describe the bit that doesn't work (but should) and what fixes it:

The original code has:

nodes_ = filter(nodes(W)) do N
    !(N isa Source)
end

which throws the error mentioned above; replacing these 3 lines with nodes_ = filter(N -> !isa(N, Source) nodes(W)) (which should be identical as far as I'm aware) fixes the issue.

Code works fine on 1.1, 1.2.

Edit: stacktrace

[ Info: Precompiling MLJ [add582a8-e3ab-11e8-2d5e-e98b27df1bc7]
ERROR: LoadError: LoadError: syntax: local variable name "N" conflicts with a static parameter
Stacktrace:
 [1] top-level scope at /Users/tlienart/.julia/packages/MLJ/XYSFt/src/composites.jl:113
 [2] include at ./boot.jl:328 [inlined]
 [3] include_relative(::Module, ::String) at ./loading.jl:1094
 [4] include at ./Base.jl:31 [inlined]
 [5] include(::String) at /Users/tlienart/.julia/packages/MLJ/XYSFt/src/MLJ.jl:1
 [6] top-level scope at /Users/tlienart/.julia/packages/MLJ/XYSFt/src/MLJ.jl:88
 [7] include at ./boot.jl:328 [inlined]
 [8] include_relative(::Module, ::String) at ./loading.jl:1094
 [9] include(::Module, ::String) at ./Base.jl:31
 [10] top-level scope at none:2
 [11] eval at ./boot.jl:330 [inlined]
 [12] eval(::Expr) at ./client.jl:433
 [13] top-level scope at ./none:3
in expression starting at /Users/tlienart/.julia/packages/MLJ/XYSFt/src/composites.jl:113
in expression starting at /Users/tlienart/.julia/packages/MLJ/XYSFt/src/MLJ.jl:88
ERROR: Failed to precompile MLJ [add582a8-e3ab-11e8-2d5e-e98b27df1bc7] to /Users/tlienart/.julia/compiled/v1.3/MLJ/rAU56.ji.
Stacktrace:
 [1] error(::String) at ./error.jl:33
 [2] compilecache(::Base.PkgId, ::String) at ./loading.jl:1253
 [3] _require(::Base.PkgId) at ./loading.jl:1013
 [4] require(::Base.PkgId) at ./loading.jl:911
 [5] require(::Module, ::Symbol) at ./loading.jl:906
@JeffBezanson
Copy link
Member

I'm guessing this could have been caused by #32623. Could you try locally reverting that commit?

@JeffBezanson JeffBezanson added bug Indicates an unexpected problem or unintended behavior compiler:lowering Syntax lowering (compiler front end, 2nd stage) labels Aug 7, 2019
@JeffBezanson JeffBezanson self-assigned this Aug 9, 2019
@DilumAluthge
Copy link
Member

@tlienart Any update on this?

@tlienart
Copy link
Contributor Author

tlienart commented Aug 16, 2019 •
edited
Loading

@DilumAluthge since it requires building Julia from source, I haven't had the time to look into it yet

@DilumAluthge
Copy link
Member

I can confirm that reverting #32623 locally fixes this error. After reverting, I was able to successfully import MLJ (and also import MLJBase and import MLJModels).

My branch with the revert is the branch da/investigate-issue-32816 on my fork (https://github.com/DilumAluthge/julia).

For posterity, here is the output of Pkg.status(mode=PKGMODE_MANIFEST):

julia> Pkg.status(mode=PKGMODE_MANIFEST)
    Status `~/.julia/environments/v1.4/Manifest.toml`
  [7d9fca2a] Arpack v0.3.1
  [9e28174c] BinDeps v0.8.10
  [b99e7846] BinaryProvider v0.5.6
  [336ed68f] CSV v0.5.11
  [324d7699] CategoricalArrays v0.5.2
  [3da002f7] ColorTypes v0.8.0
  [34da2185] Compat v2.1.0
  [9a962f9c] DataAPI v1.0.1
  [a93c6f00] DataFrames v0.18.4
  [864edb3b] DataStructures v0.17.0
  [e2d170a0] DataValueInterfaces v1.0.0
  [b4f34e82] Distances v0.8.1
  [31c24e10] Distributions v0.21.1
  [5789e2e9] FileIO v1.0.7
  [53c48c17] FixedPointNumbers v0.6.1
  [cd3eb016] HTTP v0.8.4
  [83e8ac13] IniFile v0.5.0
  [82899510] IteratorInterfaceExtensions v1.0.0
  [2d691ee1] LIBLINEAR v0.5.1
  [b1bec4e5] LIBSVM v0.3.1
  [add582a8] MLJ v0.2.5
  [a7f614a8] MLJBase v0.2.6
  [d491faf4] MLJModels v0.2.5
  [739be429] MbedTLS v0.7.0
  [e1d29d7a] Missings v0.4.1
  [bac558e1] OrderedCollections v1.1.0
  [90014a1f] PDMats v0.9.9
  [69de0a69] Parsers v0.3.6
  [2dfb63ee] PooledArrays v0.5.2
  [92933f4c] ProgressMeter v1.0.0
  [1fd47b50] QuadGK v2.0.3
  [3cdcf5f2] RecipesBase v0.7.0
  [189a3867] Reexport v0.2.0
  [cbe49d4c] RemoteFiles v0.3.0
  [ae029012] Requires v0.5.2
  [79098fc4] Rmath v0.5.0
  [6e75b9c4] ScikitLearnBase v0.5.0
  [a2af1166] SortingAlgorithms v0.3.1
  [276daf66] SpecialFunctions v0.7.2
  [2913bbd2] StatsBase v0.32.0
  [4c63d2b9] StatsFuns v0.8.0
  [3783bdb8] TableTraits v1.0.0
  [bd369af6] Tables v0.2.11
  [30578b45] URIParser v0.4.0
  [ea10d353] WeakRefStrings v0.6.1
  [2a0f44e3] Base64
  [ade2ca70] Dates
  [8bb1440f] DelimitedFiles
  [8ba89e20] Distributed
  [9fa8497b] Future
  [b77e0a4c] InteractiveUtils
  [76f85450] LibGit2
  [8f399da3] Libdl
  [37e2e46d] LinearAlgebra
  [56ddb016] Logging
  [d6f4376e] Markdown
  [a63ad114] Mmap
  [44cfe95a] Pkg
  [de0858da] Printf
  [9abbd945] Profile
  [3fa0cd96] REPL
  [9a3f8284] Random
  [ea8e919c] SHA
  [9e88b42a] Serialization
  [1a1011a3] SharedArrays
  [6462fe0b] Sockets
  [2f01184e] SparseArrays
  [10745b16] Statistics
  [4607b0f0] SuiteSparse
  [8dfed614] Test
  [cf7118a7] UUIDs
  [4ec0a83e] Unicode

@JeffBezanson
Copy link
Member

Thanks. It would really help to have a reduced case here.

@JeffBezanson JeffBezanson added this to the 1.3 milestone Aug 18, 2019
@DilumAluthge
Copy link
Member

So, uh, I might have egg on my face for this one.

The reason that this error is being thrown in MLJ version 0.2.5 is... well, the error message is correct in MLJ version 0.2.5.

If you look at line 122 of https://github.com/alan-turing-institute/MLJ.jl/blob/v0.2.5/src/composites.jl , you'll see this:

function Base.replace(W::Node, pairs::Pair...) where N

And then on lines 149 through 151, inside that same method definition:

    nodes_ = filter(nodes(W)) do N
        !(N isa Source)
    end

So the error is correct. Because we have N as a static parameter, we can't then also use N later inside the body of that method.

I cloned the MLJ repo, checked out the v0.2.5 tag, and dev-ed the package. I was able to reproduce the error. Then, I removed the where N from the end of line 122 of composites.jl, and the error went away.

The reason that @tlienart and I had such a hard time making an MWE is probably that we both were reading the source code of MLJ master. But, as you can see from looking at line 133 of https://github.com/alan-turing-institute/MLJ.jl/blob/master/src/composites.jl, the where N statement has been removed in MLJ master. It was removed by @ablaom in: JuliaAI/MLJ.jl@2b10017

In fact, I tried adding MLJ#master in a clean environment, and it works just fine.

@tlienart Are you able to reproduce this bug when adding MLJ#master in a clean package environment?

If not, we can probably close this issue as "cannot reproduce when using MLJ master and Julia master."

@c42f
Copy link
Member

c42f commented Aug 25, 2019

I managed to construct a reduced example for this:

julia> function foo(x::N) where N
           for N in x
               println(N)
           end
       end
ERROR: syntax: local variable name "N" conflicts with a static parameter

@StefanKarpinski
Copy link
Member

That seems like a correct error: you are not allowed to shadow a static parameter with a local.

@ablaom
Copy link

ablaom commented Aug 26, 2019

@DilumAluthge 's observations above suggest this is red herring. In any case the pkg triggering the original discussion (MLJ) passes tests on 1.0, 1.1., 1.2 and nightly.

Think you can probably close this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@JeffBezanson JeffBezanson

Labels
bug Indicates an unexpected problem or unintended behavior compiler:lowering Syntax lowering (compiler front end, 2nd stage)
Projects
None yet
Milestone
1.3
Development

No branches or pull requests
7 participants
@StefanKarpinski @c42f @JeffBezanson @KristofferC @DilumAluthge @tlienart @ablaom