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

test: reproduce issue #1748 #1750

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

test: reproduce issue #1748 #1750

wants to merge 2 commits into from

Conversation

voodoos
Copy link
Collaborator

@voodoos voodoos commented Apr 12, 2024

No description provided.

@voodoos voodoos added this to Planned for backporting in Backport to 500 via automation Apr 12, 2024
@voodoos
Copy link
Collaborator Author

voodoos commented Apr 12, 2024

It looks like Merlin is not performing the lookup in the correct environment:

  # 0.01 locate - from_string
  looking for the source of 'Import' (prioritizing .ml files)
  # 0.01 locate - lookup
  lookup in module namespace
  # 0.01 locate - env_lookup
  found: 'Import!.Import' in namespace module with uid Bar.1

@xvw
Copy link
Collaborator

xvw commented Apr 12, 2024

Is this ready for review?
The fix seems good (thanks to the test-suite)

@liam923
Copy link
Contributor

liam923 commented Apr 12, 2024

This is great, it seems to fix the bug I originally reported. Thanks for the quick turnaround.

I did find a new issue that it causes, however. Once some wrapping modules are added, jumping from an implementation now causes a Not in environment error. See the test I attached below that demonstrates this:

homonyms.t.tar.gz

@voodoos
Copy link
Collaborator Author

voodoos commented Apr 15, 2024

This is great, it seems to fix the bug I originally reported. Thanks for the quick turnaround.

I did find a new issue that it causes, however. Once some wrapping modules are added, jumping from an implementation now causes a Not in environment error. See the test I attached below that demonstrates this:

homonyms.t.tar.gz

Right, I think we will need a different approach. I use an empty env which explains that error.

@voodoos voodoos marked this pull request as draft April 15, 2024 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Backport to 500
Planned for backporting
Development

Successfully merging this pull request may close these issues.

None yet

3 participants