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

Find references is currently broken on 1.10 #257

Closed
axelson opened this issue May 21, 2020 · 6 comments
Closed

Find references is currently broken on 1.10 #257

axelson opened this issue May 21, 2020 · 6 comments

Comments

@axelson
Copy link
Member

axelson commented May 21, 2020

textDocument/references is currently broken when running on 1.10. This is most likely related to xref changes when compiler tracing was implemented in Elixir. Still need to test if compiling elixir-ls on 1.10 will have an impact.

@axelson axelson pinned this issue May 21, 2020
@axelson
Copy link
Member Author

axelson commented May 22, 2020

This might be considered a bug in Elixir 1.10, so if someone can reproduce the bug in Mix.Tasks.Xref (as used in ElixirSense) it could be reported upstream.

@lukaszsamson
Copy link
Collaborator

I tried master build and running on 1.10 as well as 0.4 build on 1.7 and running on 1.10 and in both combinations it worked.

@axelson
Copy link
Member Author

axelson commented Jun 2, 2020

Hmmm, I'll do some more investigation to see if there's a different root cause.

@msaraiva
Copy link
Collaborator

msaraiva commented Jun 5, 2020

I'm using v1.10.3 and it's working fine. It would nice to get more information from people experiencing this problem, like:

  • Is there any error message in ElixirLS console?
  • Does the problem happens in all projects or maybe just in umbrellas?
  • Does running Mix.Tasks.Xref.calls() in a iex -S mix console works?

@lukaszsamson what do we need to do regarding elixir-lsp/elixir_sense#80 to make it feasible? As far as I remember, it requires a stateful tracer, right? If that's the case, I don't think we'll have a choice but creating a stateful application for ElixirSense. Editors that don't use elixir-ls will have to find a way to use elixir-ls or roll their own solution.

Unfortunately, we can't keep using deprecated API any longer, IMO. Besides, using the tracer will allow us to gradually simplify some of the most complex logic we have, making the code less sensitive to Elixir updates.

Also, having a stateful process around opens new possibilities for optimization and new features that might require access to previously fetched information (e.g. the list of all loaded modules, etc.)

@axelson WDTY?

@lukaszsamson
Copy link
Collaborator

it requires a stateful tracer, right?

Yes, but it doesn't necessarily be managed by elixir_sense, it may be managed by elixir-ls and injected. The bigger problem is that it requires elixir >= 1.10 and I'm not sure it will even work when compiled with 1.7.

Also, having a stateful process around opens new possibilities for optimization and new features

A few modules could use a cache

@axelson
Copy link
Member Author

axelson commented Jun 5, 2020

Hmmm, it's seeming like I was wrong about this issue. Since we can't reproduce it I'm going to go ahead and close it, perhaps there was some other issue that was since fixed. If we can reproduce it in the future we can re-visit.

Regarding elixir-lsp/elixir_sense#80 the issue is (as Łukasz mentioned) that it requires elixir 1.10+ and it's too early to require Elixir 1.10. And it most likely be a significant undertaking to maintain dual implementations where we'd use compiler tracing if it is available, but ignore it if it is not available (and we'd have to solve the packaging problem (#115) as well, or at least have end-users automatically compile ElixirLS).

@axelson axelson closed this as completed Jun 5, 2020
@axelson axelson unpinned this issue Jun 5, 2020
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

No branches or pull requests

3 participants