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

Linking issue with Template Haskell that depends on C code #365

Closed
dpvanbalen opened this issue Aug 31, 2020 · 24 comments
Closed

Linking issue with Template Haskell that depends on C code #365

dpvanbalen opened this issue Aug 31, 2020 · 24 comments
Labels
component: ghcide type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..

Comments

@dpvanbalen
Copy link

On Windows, when trying to use the VSCode plugin on accelerate. HLS works as expected on other codebases.

I get the following, identical, errormessage on the Bool.hs, Either.hs, Maybe.hs and Ordering.hs files:

Program error:
ByteCodeLink: can't find label
During interactive linking, GHCi couldn't find the following symbol:
__cmd_line_flags
This may be due to you not asking GHCi to load extra object files,
archives or DLLs needed by your current session. Restart GHCi, specifying
the missing library using the -L/path/to/object/dir and -lmissinglibname
flags, or simply by naming the relevant files on the GHCi command line.
Alternatively, this link failure might indicate a bug in GHCi.
If you suspect the latter, please send a bug report to:
glasgow-haskell-bugs@haskell.org

All four of these files apply the TH function mkPattern from Pattern/TH.hs on these Prelude datatypes.

As a result, it seems (looking at the verbose output in VSCode) that HLS always responds to queries in this package (hover, go to definition, etc), even unrelated to these patterns, with null or [].

@jneira jneira added status: needs repro type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc.. labels Aug 31, 2020
@dpvanbalen
Copy link
Author

dpvanbalen commented Sep 3, 2020

Some more details:

Apparently, this issue has to do with the TH code depending on C code, which means it must be built before compiling the TH module. It seems that HLS somehow messes up this ordering, whilst stack repl does work. Apparently Cabal has the same problem. Adding an hie.yaml did not help either.

This issue also persists using an ubuntu VM via WSL2, which further indicates it's not a Windows problem.

I also just noticed that, in the modules that don't (transitively) depend on this TH function executing (in particular, the module where the TH function is defined), hls does work!

@dpvanbalen dpvanbalen changed the title Linking issue with Template Haskell on Windows Linking issue with Template Haskell that depends on C code Sep 3, 2020
@tmcdonell
Copy link

Minimal repro: https://github.com/tmcdonell/hls-bug

@pepeiborra
Copy link
Collaborator

pepeiborra commented Sep 9, 2020

There is no cradle in your repro. Could you please define one and test again? Test both with the Cabal and Stack cradles.

For this to work, the C object needs to be listed in the flags provided by hie-bios for the cradle

@dpvanbalen
Copy link
Author

  • stack repl: works
  • cabal repl: doesn't work
  • simple cradle (both stack and cabal): don't work

where with 'simple cradle' I mean an hie.yaml file containing
cradle: stack: component: "hls-bug:lib"
or
cradle: cabal: component: "lib:hls-bug.

What flags could list the C file?

@jneira
Copy link
Member

jneira commented Sep 29, 2020

Maybe it will be fixed with TH full support: haskell/ghcide#836

@dpvanbalen

What flags could list the C file?

The ghc flags that the component hie-bios provides to haskell-language-server, needed for load those files .
does cabal repl/stack repl work for the project? It it is so please include the log of executing haskell-language-server-wrapper --debug in the project root dir

@wz1000
Copy link
Collaborator

wz1000 commented Sep 29, 2020

I don't think this would work right now, I think we need to compile the foreign files like this:

https://hackage.haskell.org/package/ghc-8.10.1/docs/src/DriverPipeline.html#local-6989586621681024042

@jneira jneira added component: ghcide status: blocked Not actionable, because blocked by upstream/GHC etc. labels Sep 29, 2020
@jneira
Copy link
Member

jneira commented Nov 17, 2020

This would need an issue in ghcide

pepeiborra pushed a commit that referenced this issue Dec 27, 2020
* Add a plugin type

* Add a helper to construct codeAction values

* Remove a redundant $
@robbert-vdh
Copy link

robbert-vdh commented Feb 21, 2021

Any new insight on what needs to be done for this to work properly? HLS is still completely nonfunctional in any module that either directly or indirectly imports a module that uses calls Template Haskell code defined in some other module in a package that uses the FFI.

In the example reported in the OP the particular module that exposes __cmd_line_flags is not part of the dependency chain of the module that HLS is showing an error for. I've visualized the module hierarchy of Accelerate with graphmod here. The modules HLS is having issues with are the four blue Pattern, Maybe, Ordering and Bool modules on the left, and __cmd_line_flags is defined and used in Data.Array.Accelerate.Debug.Internal.Flags (one of the light green ones slightly to the right of the middle of the graph).

@wz1000
Copy link
Collaborator

wz1000 commented Feb 21, 2021

indirectly imports a module that uses calls Template Haskell code defined in some other module in a package that uses the FFI.

Are you talking about package part of cabal/hackage dependencies, or ones which are local to your project?

@robbert-vdh
Copy link

These modules are all local to the project/package. If you were to move one of the modules that's causing issues from Accelerate to some other external library that links to Accelerate then import the module from there then there are no issues.

@wz1000
Copy link
Collaborator

wz1000 commented Feb 21, 2021

If you want a temporary workaround for now, perhaps you use #ifdef __GHCIDE__ ... to stub out the parts of the code that depend on FFI.

robbert-vdh added a commit to robbert-vdh/accelerate that referenced this issue Feb 21, 2021
Without these stubs HLS wouldn't be able to provide completion in any
module that directly or indirectly includes any of the
'Data.Array.Accelerate.Pattern.*' modules.

haskell/haskell-language-server#365
@robbert-vdh
Copy link

robbert-vdh commented Feb 21, 2021

That's a great idea! I've added some stubs, and now HLS works again as expected. 🎉

Of course it would be ideal if adding stubs/using the CPP wasn't necessary, but given that our use case is probably fairly rare and fixing it is likely also non-trivial this workaround works great for the time being. Thanks a lot!

robbert-vdh added a commit to robbert-vdh/accelerate that referenced this issue Feb 21, 2021
Without these stubs HLS wouldn't be able to provide completion in any
module that directly or indirectly includes any of the
'Data.Array.Accelerate.Pattern.*' modules.

haskell/haskell-language-server#365
robbert-vdh added a commit to robbert-vdh/accelerate that referenced this issue Feb 21, 2021
Without these stubs HLS wouldn't be able to provide completion in any
module that directly or indirectly includes any of the
'Data.Array.Accelerate.Pattern.*' modules.

haskell/haskell-language-server#365
@nh2
Copy link
Member

nh2 commented Nov 23, 2021

I'm suffering from this too.

My project (many projects, in fact), use inline-c and inline-c-cpp to call foreign code.

They only compile with -fobject-code, as explained on https://github.com/fpco/inline-c/tree/master/inline-c#ghci

Thus I get with HLS:

  ByteCodeLink: can't find label
  During interactive linking, GHCi couldn't find the following symbol:
  inline_c_My_Module_4

This bug is labelled as status: blocked upstream. Where is the upstream issue?

@nh2
Copy link
Member

nh2 commented Nov 23, 2021

we need to compile the foreign files like this:

https://hackage.haskell.org/package/ghc-8.10.1/docs/src/DriverPipeline.html#local-6989586621681024042

@wz1000 Were would that have to be done?

@nh2
Copy link
Member

nh2 commented Nov 23, 2021

Also, is there a non-CPP equivalent to #ifndef __GHCIDE__ (e.g. a TH one, or an environment variable) that could be used to make inline-c notice that it's running under HLS, so that it can emit stubs instead of foreign calls in that case?

@jneira
Copy link
Member

jneira commented Nov 23, 2021

@nh2

This bug is labelled as status: blocked upstream. Where is the upstream issue?

Sorry the label is obsolete as the issue was created when ghcide lived in another repo. I wanted to mean hls was blocked on a ghcide change and i noted

This would need an issue in ghcide

But the issue was never created 😟

Removing the label as ghcide lives here now and it is where the fix mentioned by @wz1000 should be done

@jneira jneira removed the status: blocked Not actionable, because blocked by upstream/GHC etc. label Nov 23, 2021
@jneira
Copy link
Member

jneira commented Nov 23, 2021

Also, is there a non-CPP equivalent to #ifndef __GHCIDE__ (e.g. a TH one, or an environment variable) that could be used to make inline-c notice that it's running under HLS, so that it can emit stubs instead of foreign calls in that case?

Not sure if it would work for you use case but you can create a runtime check from a cpp one:

usingGhcide :: Bool
#ifdef __GHCIDE__
usingGhcide = True
#else
usingGhcide = False
#endif

@nh2
Copy link
Member

nh2 commented Nov 29, 2021

@jneira I have made a draft PR to inline-c to implement this workaround, emitting stubs instead of calls to foreign imports.

Could you have a look at it?

fpco/inline-c#128

It cannot use the CPP #ifdef __GHCIDE__, because that gets evaluated too early (when the inline-c package is compiled, where ghcide is off).
It needs to be evaluated at splice time, where CPP is not available.

So I'm determining it by reading an environment variable also called __GHCIDE__.

Do you agree with this approach?

If yes, could you make HLS set this __GHCIDE__ environment variable, so that this info is available to TH?

@jneira
Copy link
Member

jneira commented Nov 29, 2021

@nh2 thanks for working in a workaround.
Given the proper solution would be the described by @wz1000 above i would try to reuse an existing env var if any or at least try to use a new env var which could be useful generically (GHCIDE_VERSION or something alike?)

@nh2
Copy link
Member

nh2 commented Nov 29, 2021

Given the proper solution would be the described by @wz1000 above

@jneira Just to confirm, that effort hasn't been started yet, right?

Also, there might still be a tradeoff: Compiling C++ files can be very slow (e.g. up to 30 seconds if you include complex header files like from the cgal library, as I do). So it'd be nice to have a mode where stubs are emitted nevertheless.

i would try to reuse an existing env var if any or at least try to use a new env var which could be useful generically (GHCIDE_VERSION or something alike?)

Sounds good! Are there env vars that already exist that I could use?

@jneira
Copy link
Member

jneira commented Nov 29, 2021

@jneira Just to confirm, that effort hasn't been started yet, right?

Also, there might still be a tradeoff: Compiling C++ files can be very slow (e.g. up to 30 seconds if you include complex header files like from the cgal library, as I do). So it'd be nice to have a mode where stubs are emitted nevertheless.

no, it has not started afaik
so I think the workaround makes sense, moreover if there are perf related considerations

and to have a way to write code ghcide aware at runtime could have other uses (not sure if they will be clean ones though 😉)

i would try to reuse an existing env var if any or at least try to use a new env var which could be useful generically (GHCIDE_VERSION or something alike?)

Sounds good! Are there env vars that already exist that I could use?

I did a quick search and did not find anything but maybe @pepeiborra knows about some suitable env var

@pepeiborra
Copy link
Collaborator

I'm not aware of any env var that you can use to detect HLS currently.

@jneira
Copy link
Member

jneira commented Jan 31, 2022

I am gonna close this issue as all compiler crashes seems to have the same root cause:

If any of you think the issue should not be included generically feel free to reopen it (with a brief explanation if possible)
Thanks all!

@nh2
Copy link
Member

nh2 commented Sep 29, 2023

If any of you think the issue should not be included generically feel free to reopen it (with a brief explanation if possible)

@jneira I think we have to re-open this one:

I'm now on the much newer HLS in NixOS 23.05 which has the fixes from #365 (comment) in, but it is still required to do the env var based symbol stubbing for inline-c (fpco/inline-c#128).

I believe this is because

Given the proper solution would be the described by @wz1000 #365 (comment)

@jneira Just to confirm, that effort hasn't been started yet, right?

no, it has not started afaik

is still the case.

So I think we should make HLS set the env var as discussed in #365 (comment) and above, and that should close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: ghcide type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..
Projects
None yet
Development

No branches or pull requests

7 participants