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

Type info suddenly stops being returned from ide-backend #88

Closed
jwiegley opened this issue Jun 23, 2013 · 17 comments
Closed

Type info suddenly stops being returned from ide-backend #88

jwiegley opened this issue Jun 23, 2013 · 17 comments
Labels

Comments

@jwiegley
Copy link

I've created a simple project with three modules:

module Foo where

import Bar

foo = bar >> bar

foobar = putStrLn "Baz"
module Bar where

bar = putStrLn "Hello, world!"
module Baz where

import Foo
import Bar

baz = foobar >> foo >> bar

If I keep switching modules, and clicking on identifier names, I see type info for those names. But if I edit the code such that there is a syntax error, and then fix that error, it's possible (quite easily, in fact), to get into a state where no type info is returned for any identifier anymore.

It's not that an exception is happening either. I'm calling getSpanInfo, and then calling function it returns, and I'm getting back an empty list:

2013-06-23 07:23:21.25541 UTC: IR: >typeInfoForRange:3@46 green
2013-06-23 07:23:21.255554 UTC: IR: <typeInfoForRange:3@46
23/Jun/2013:02:23:21 -0500 [Debug] Command Result: Success NoTypeInfo @(learning-site-0.0.0:Handler.Fay ./Handler/Fay.hs:27:11)

This state is very easy to get one's self into, and there seems to be no simple or consistent way to "undo it". It presents to the user as if our IDE simply stops giving type information or allowing the user to jump to function definitions within the project.

This is, I believe, the source of the problem reported in fpco/fpco#1681.

Pinging @snoyberg @Mikolaj

@ghost ghost assigned edsko Jun 23, 2013
@jwiegley
Copy link
Author

This code reproduces the bug, giving the following unexpected output:

Vulcan:~/fpco/isolation-runner/FP/IsolationRunner {master} $ ./test
[(Baz.hs@6:7-6:13,foobar (VarName) :: IO () defined in main:Foo at Foo.hs@7:1-7:7 (imported from gitlib-cross-1.0.1:Foo at Baz.hs@3:1-3:11))]
[]
[]

I am OK with the empty list in the second result. I am not OK with it in the third result, where the typo info should match that returned in the first result.

Here's the test, written against ide-backend alone:

{-# LANGUAGE OverloadedStrings #-}

module Main where

import IdeSession

main = do
    sess <- initSession defaultSessionConfig
                { configDir             = "."
                , configGenerateModInfo = True
                }

    let cb = \_ -> return ()
        update = flip (updateSession sess) cb

    update $ updateCodeGeneration True
    update $ updateStdoutBufferMode (RunLineBuffering Nothing)
    update $ updateStderrBufferMode (RunLineBuffering Nothing)

    update $ updateModule "Foo.hs"
        "module Foo where\n\
        \\n\
        \import Bar\n\
        \\n\
        \foo = bar >> bar\n\
        \\n\
        \foobar = putStrLn \"Baz\"\n"

    update $ updateModule "Bar.hs"
        "module Bar where\n\
        \\n\
        \bar = putStrLn \"Hello, world!\"\n"

    update $ updateModule "Baz.hs"
        "module Baz where\n\
        \\n\
        \import Foo\n\
        \import Bar\n\
        \\n\
        \baz = foobar\n"

    getSourceErrors sess

    gif <- getSpanInfo sess
    print $ gif "Baz" (SourceSpan "Baz.hs" 6 8 6 9)

    update $ updateModule "Baz.hs"
        "module Baz where\n\
        \\n\
        \import Foo\n\
        \import Bar\n\
        \\n\
        \baz = foobar >>>> foo >> bar\n"

    getSourceErrors sess

    gif <- getSpanInfo sess
    print $ gif "Baz" (SourceSpan "Baz.hs" 6 8 6 9)

    update $ updateModule "Baz.hs"
        "module Baz where\n\
        \\n\
        \import Foo\n\
        \import Bar\n\
        \\n\
        \baz = foobar >> foo >> bar\n"

    getSourceErrors sess

    gif <- getSpanInfo sess
    print $ gif "Baz" (SourceSpan "Baz.hs" 6 8 6 9)

@snoyberg
Copy link
Member

@edsko @Mikolaj This is a major issue for us, I'd like to get a fix for this by the time we have our beta release this week. Please spend up to 1 day on initial work on fixing this, and if it looks like the fix will take more time, please be in touch with me ASAP.

We do not want to have to include a bunch of other changes to get a fix for this, so please provide this as a patch against the code we're currently building against (https://github.com/fpco/ide-backend/tree/47b556895bfb2fd9ef071f7dbb6f8923870a15e6). If you have any questions, please let us know.

@jwiegley
Copy link
Author

I should also note that I first witnessed this behavior about two weeks ago, but wrote it off to our isolation container running slowly in our testing environment. So I don't believe this is something new, just something that's gone unnoticed.

@mgsloan
Copy link
Contributor

mgsloan commented Jun 23, 2013

It'd also be good to check that asking for info about modules that aren't in compilation doesn't cause something like this. While I have no concrete evidence that this is a problem, I just fixed a frontend bug that would cause info queries for a "Main" module that's not included in compilation. Interacting with "Main" modules seemed to be helpful in reproducing this.

@jwiegley
Copy link
Author

@mgsloan I'm not sure what you mean in suggesting that "Main" might somehow be special. The test case above demonstrates the problem with having any module named "Main". I don't think that name is special to the ide-backend at all.

@mgsloan
Copy link
Contributor

mgsloan commented Jun 23, 2013

@jwiegley Right, "Main" is only special to the frontend. Multiple "Main" modules are allowed. The bug was that type info queries were allowed for "Main" modules even when they weren't the current target (therefore excluded).

Anyway, I don't want to distract from the matter at hand - fixing your test case is definitely the priority.

@edsko
Copy link
Collaborator

edsko commented Jun 24, 2013

Looking at this now.

edsko added a commit that referenced this issue Jun 24, 2013
@edsko edsko closed this as completed in 88c52d1 Jun 24, 2013
@edsko
Copy link
Collaborator

edsko commented Jun 24, 2013

Fixed this issue (which was due to the fact that ordering a list of ghc's ModuleNames is not the same as ordering the corresponding Strings, as the ModuleName wraps around FastString and are compared on their hash). In the process however discovered two further issues: #89, which is an optimization and not urgent, and #90, which is a bug (albeit a slightly strange one: the set of modules exported by packages should be mutually disjoint; this bug arises when the "home" package, the one currently being compiled, exports a module that is also exported by one of the installed packages).

@jwiegley
Copy link
Author

@edsko Can I patch the version of ide-backend that I'm using by just cherry picking those two commits?

@edsko
Copy link
Collaborator

edsko commented Jun 24, 2013

@jwiegley I'm not sure which two commits you are referring to? I've pushed the patch directly to your temporary-pkgdir-hack branch (and then cherry-picked it for the experimental and master branches).

@jwiegley
Copy link
Author

@edsko Ok, thanks, I'm integrating it now!

@jwiegley
Copy link
Author

@edsko Your update to temporary-pkgdir-hack is failing one of the unit tests:

  Consistency of IdMap/explicit sharing cache through multiple updates (#88): [Failed]
ERROR: Prelude.head: empty list

         Test Cases    Total        
 Passed  128           128          
 Failed  1             1            
 Total   129           129          
Test suite ghc-errors: FAIL

I've confirmed that this does not fail prior to your changes. Can you please rectify this so that I can deploy it?

@jwiegley jwiegley reopened this Jun 26, 2013
@Mikolaj
Copy link
Collaborator

Mikolaj commented Jun 26, 2013

I can't reproduce the test failure. Have you done 'cabal install' inside the server/ directory?

@jwiegley
Copy link
Author

@Mikolaj We aren't using a version of ide-backend which has a cabal file in the server directory.

@snoyberg
Copy link
Member

@Mikolaj @edsko Thanks for addressing this one so quickly. We're building it on our staging server, and will let you know after we've tested there.

@Mikolaj
Copy link
Collaborator

Mikolaj commented Jun 26, 2013

Some bits of the IRC log, to share with all interested parties and note down some things to verify later on.

07:27 < johnw> I'm even getting a failure in the good version now
07:27 < johnw> Build licenses from NamedFieldPuns: [Failed]
07:27 < johnw> licensesWarns length
07:27 < johnw> expected: 367
07:27 < johnw> but got: 505
07:28 * mikolaj checks
07:29 < johnw> i'll see if the new code has the same error, which should be fine
07:29 < johnw> which we're not using the license stuff yet
07:31 < mikolaj> yep, the license tests are quite fragile, so they may faile easily with cherry-picking, etc.
07:31 < mikolaj> running the tests
07:40 < mikolaj> hmm, I'm not getting the licenses errors, so probably the tests are even more fragile than I expected (e.g., they depend on the length of the LICENSE file for the particular versions of package you use)
07:40 < mikolaj> *of packages
07:42 * mikolaj notes down to work on the licence tests in the future
07:43 < johnw> yeah, the failure is the same

09:21 < johnw> do these failures look benign to you: https://gist.github.com/5865406
09:22 < johnw> we're seeing them in our test-fpco-staging build
09:52 < mikolaj> johnw: yes, they look benign; but I will let edsko know just in case
09:53 < mikolaj> (the first is probably from no haddock generated and isntalled for parallel)
09:53 < mikolaj> (the second is just different version of yesod-routes)
09:54 < mikolaj> (plus no haddock for it, probably)
09:55 < mikolaj> some tests are context sensitive (e.g., the license tests, apparently) and we will have to work on adjusting them for your contexts, too

@edsko
Copy link
Collaborator

edsko commented Jun 26, 2013

Like @Mikolaj said: the first failure is probably due to the absence of a .haddock file, the second is just a version mismatch. Version mismatches are already abstracted away from in later versions of ide-backend; not 100% sure how to deal with the absence of .haddock files -- unless there is good reason why these files would be absent, I would say that it's ok to assume they exist (after all, ide-backend relies on them to provide home module information).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants