LanguageServer.jl icon indicating copy to clipboard operation
LanguageServer.jl copied to clipboard

global consts are not indexed properly

Open mfiano opened this issue 2 years ago • 10 comments

# scalar.jl
module Scalars

const Scalar = Union{Signed,AbstractFloat}

export Scalar

end
# vector.jl
module Vectors

import StaticArrays as SA

using Scalars

abstract type BaseVector{N,T<:Scalar} <: SA.FieldVector{N,T} end # Missing reference: Scalar

struct Vector2{T<:Scalar} <: BaseVector{2,T} # Missing reference: Scalar
    x::T
    y::T
end

end

@fredrikekre had mentoned that they could reproduce this issue on Zulip and that global consts are not indexed properly.

mfiano avatar Jul 07 '22 17:07 mfiano

I also noticed that it is only when the const is from another file, for example:

$ cat src/OtherFile.jl
module OtherFile
    const O = Int
    export O
end

$ cat src/TestPackage.jl
module TestPackage

module SameFile
    const S = Int
    export S
end
using .SameFile

include("OtherFile.jl")
using .OtherFile

function foo()
    s = S                   # <---- No error here
    o = O                   # <---- Missing reference: O
    return s, o
end

end

here there is only a missing reference for O, not for S.

fredrikekre avatar Jul 08 '22 12:07 fredrikekre

I also noticed that it is only an issue when the const is from another file, and it is brought in using using. import Mod: foo or import Mod as M allows the use of foo or M.foo without errors.

mfiano avatar Jul 08 '22 18:07 mfiano

This is a bigger problem than I thought. Just adding a constant type alias and trying to define a constructor of the same name produces error diagonostics, however, the code loads and evaluates them correctly.

I had to modify my editor config to hide all diagnostics in the project I'm working on, as more than half of my buffers had diagnostic text overlays distracting the readability of the code.

@fredrikekre Note that this also occurs if the const is in the same module and file as the function of the same name.

mfiano avatar Jul 09 '22 10:07 mfiano

This is a bigger problem than I thought. Just adding a constant type alias and trying to define a constructor of the same name produces error diagonostics, however, the code loads and evaluates them correctly.

Isn't this exactly the problem from OP?

Note that this also occurs if the const is in the same module and file as the function of the same name.

Example?

fredrikekre avatar Jul 09 '22 11:07 fredrikekre

Isn't this exactly the problem from OP?

Not quite. The OP was about a) consts defined in another file, and b) used as type parameters.

I also noticed that it is only when the const is from another file[...]

This is likely the same issue, but I was able to reproduce it in the same file, which you weren't able to reproduce. In this case, I'm defining type aliases, and then defining outer constructors of the same name, which is semantically valid according to the compiler, just not with LS.jl. A picture sums it up:

mfiano avatar Jul 09 '22 11:07 mfiano

b) used as type parameters.

How the undefined variable is used doesn't really matter is what I mean, it is undefined regardless.

Can you post copy-paste (a minimal) example instead of posting a screenshot?

fredrikekre avatar Jul 09 '22 11:07 fredrikekre

I am able to reproduce it with a minimal base module as such:

module Mod

using StaticArrays

const Vector2 = SVector{2,Float64}

Vector2(s::Real) = fill(s, Vector2)

end

mfiano avatar Jul 09 '22 11:07 mfiano

Okay, that doesn't give diagnostics for missing reference though, for me it gives Cannot define function ; it already has a value. so that is a separate issue. (Also note that Vector2 isn't a proper type here, just an alias, so defining constructors for it is type-piracy since you add constructors to SVector{2,Float64}.)

fredrikekre avatar Jul 09 '22 11:07 fredrikekre

Yes, it is a different diagnostic. About type piracy, yes, noted. I am actively trying to avoid that. This was just something I came across accidentally while trying things with Sukera. I am definitely not sticking with it.

I should have mentioned the diagnostic message was different. I thought it may be related to this issue in some way, since the problem was with constants not being indexed properly. I apologize for forgetting to mention that detail.

mfiano avatar Jul 09 '22 11:07 mfiano

You can open a separate issue with https://github.com/julia-vscode/LanguageServer.jl/issues/1123#issuecomment-1179527072

fredrikekre avatar Jul 09 '22 11:07 fredrikekre