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

Add sourcepath to IDE setup #3949

Merged
merged 38 commits into from
Feb 12, 2018
Merged

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Jan 30, 2018

This allows the compiler to succeed even if the project is not fully built.
It also gives navigation capability into directly dependent files.

@odersky odersky requested a review from smarter January 30, 2018 15:56
@odersky odersky force-pushed the ide-sourcepath branch 2 times, most recently from fc8c8df to d1db0dc Compare January 31, 2018 22:59
@odersky
Copy link
Contributor Author

odersky commented Jan 31, 2018

@allanrenucci Can you figure out what's wrong with test?

@allanrenucci
Copy link
Contributor

@odersky The issue is when compiling with -from-tasty. You can reproduce locally:

# Make sure output dir is empty
$ rm -rf out/*
$ sbt
> dotc -d out tests/run/hello.scala
> dotc -from-tasty -classpath out Test -Xprint:all

The second sbt command does not compile anything. This line returns None: https://github.com/dotty-staging/dotty/blob/d1db0dc9bd82bab724361f756d5d487fed926ba0/compiler/src/dotty/tools/dotc/fromtasty/ReadTastyTreesFromClasses.scala#L69

@odersky
Copy link
Contributor Author

odersky commented Feb 1, 2018

FromTastyTests seems to be flakey (compilation order dependent?). I can't repeat test failures on the CI locally, and vice versa I get failures locally that are not an issue on the CI. @nicolasstucki can you look into this at some point?

@odersky
Copy link
Contributor Author

odersky commented Feb 1, 2018

@allanrenucci I don't think the test failure is explained by the FromTasty tests. I mean your diagnosis was correct, but this did not cause a test failure. Even after the problem is fixed, we still fail in the same way. It's not in FromTasty. It seems towards the end of the run. Here is what we got:

./project/scripts/sbt ';dotc tests/pos/sbtDotrTest.scala -d out/scriptedtest1/; dotc -from-tasty -classpath out/scriptedtest1/ -d out/scriptedtest2/ dotrtest.Test; dotr -classpath out/scriptedtest2/ dotrtest.Test'
1159s
11196
Error: Could not find or load main class dotrtest.Test
1177s
11197

  • cat sbtdotr2.out
    1177s
    11198
    [info] Loading project definition from /tmp/1/project/project
    1177s
    11199
    [info] Loading project definition from /tmp/1/project
    1177s
    11200
    [info] Resolving key references (16135 settings) ...
    1177s
    11201
    [info] Set current project to dotty (in build file:/tmp/1/)
    1177s
    11202
    [warn] Multiple main classes detected. Run 'show discoveredMainClasses' to see the list
    1177s
    11203
    [warn] Multiple main classes detected. Run 'show discoveredMainClasses' to see the list
    1177s
    11204
    [info] Running dotty.tools.dotc.Main -classpath /tmp/1/library/target/scala-2.12/dotty-library_2.12-0.7.0-bin-SNAPSHOT-nonbootstrapped.jar tests/pos/sbtDotrTest.scala -d out/scriptedtest1/
    1177s
    11205
    [success] Total time: 4 s, completed Feb 1, 2018 2:18:55 PM
    1177s
    11206
    [warn] Multiple main classes detected. Run 'show discoveredMainClasses' to see the list
    1177s
    11207
    [warn] Multiple main classes detected. Run 'show discoveredMainClasses' to see the list
    1177s
    11208
    [info] Running dotty.tools.dotc.Main -classpath out/scriptedtest1/:/tmp/1/library/target/scala-2.12/dotty-library_2.12-0.7.0-bin-SNAPSHOT-nonbootstrapped.jar -from-tasty -d out/scriptedtest2/ dotrtest.Test
    1177s
    11209
    [success] Total time: 2 s, completed Feb 1, 2018 2:18:57 PM
    1177s
    11210
    [warn] Multiple main classes detected. Run 'show discoveredMainClasses' to see the list
    1177s
    11211
    [warn] Multiple main classes detected. Run 'show discoveredMainClasses' to see the list
    1177s
    11212
    [success] Total time: 0 s, completed Feb 1, 2018 2:18:58 PM
    1177s
    11213
  • grep -e 'dotr test ok' sbtdotr2.out
    1177s
    11214
    failed output check
    1177s
    11215
  • echo 'failed output check'
    1177s
    11216
  • exit -1

@odersky
Copy link
Contributor Author

odersky commented Feb 1, 2018

I see, it was a from tasty test after all. But I am at the end of my patience with them now. Somebody else will have to take this over and re-instantiate the failing tests. I never wanted to get involved with them in the first place, but they were so entangled with source trees that I had no choice.

@odersky
Copy link
Contributor Author

odersky commented Feb 2, 2018

Current status: We can now construct trees from source, it's no longer required that the project is built so that we can read the Tasty trees.

Also, we have now a cool find-references, which works very fast even for large code bases. For example it finds all 3360 references to "Context" in the Dotty project in under 3 seconds. Smaller sets take much less than a second.

@odersky
Copy link
Contributor Author

odersky commented Feb 2, 2018

Current status: We can now also navigate to overridden symbols.

@nicolasstucki
Copy link
Contributor

I will fix the from tasty tests later. We could have issues that come from compilation order. The error that @allanrenucci pointed out means that the class is not found or cannot be loaded from the classpath.

This allows the compiler to succeed even if the project is not fully built.
It also gives navigation capability into directly dependent files.
Strangely, it seems that at least for simple dependencies, the IDE already knows about
classes even if class name != source file name. @smarter Do you have an idea why?
Use less state: just a single field that can be either a tree or a tree provider
This should make it easy to integrate sourfe files in IDE.

The refactoring required a major refactoring in ReadTastyTreesFromClasses, which
was a mess before. Hopefully it's not right. There are still "spurious" warnings
coming from classes loaded twice. These will require a refactoring of FromTastyTest
to avoid. They do not happen if the FromTasty frontend is invoked stand-alone.
1. Since Symbol.tree no longer automatically reads positions (it should not, really,
that's what we have contexts for!), ReadPositions has to be turned on in therelevant tests.

2. Disabled simpleClass which fails again because wrong class file ends up being decompiled.
... so that it can be shared with SourcefileLoader in the future.
Various tweaks necessary so that symbols can be loaded from source.
After having implemented the interface between IDE and SymbolLoaders it turns out
lateUnits is not needed, and lateFiles is used only internally.
 - Don't fail if a class file is not found
 - Consult name tables of Tasty info for a possible match
   before loading tree.
 - Do the same for rename
Found to be usused by invoking findReferences, yay!
Don't parse a new tree, since this generates a new set of symbols which are
hard to correlate with the old ones.
The previous version matched too much, as it as not comparing signatures.
We no longer need to relate late-loaded symbols that should be the same as
existsing ones, so the method can be simplified.
Simple comparison is enough, since we no longer have to deal with
late-loaded symbols.
Lazy annotations need not be forced when cleaning up since any
future evaluation wil be in a future context, so they cannot contain
dangling completers.
We got into problems when the symbols were entered in one run and
the tree was demanded in a subsequent run. In that case we'd need
to time-travel back which the framework does not really support.

The new scheme makes sure the typechecking is done at the same run
as when the symbols were entered. It either immediately typechecks
them, or if called during a compile, waits until the end of the
compile. The latter twist is done so that we do not cause new cycles.
For modules we sometimes get the module class instead of the module
val as symbol. In that case we should not filter references by the module class
name but by the module name.
Despite what the comment said they were not skipped but traversed in their
entirety, which led to "interesting" find-references results. Now only
the call is traversed.
The previous cast can fail if applyOverloaded is used outside a compilation
run. This can happen in the IDE, when annotations are forced. Reconstructing
applications from Scala2 pickles calls applyOverloaded.
These are now shown in "definitions" if the cursor position
is on a definition.
Following "name all the things" mantra.
We need that to be able to establish precise contexts.
Collect them all in NamerContextOps. Previously some were also in Typer and
in Context itself.
@smarter smarter merged commit d93a8eb into scala:master Feb 12, 2018
@Blaisorblade Blaisorblade deleted the ide-sourcepath branch August 20, 2018 18:48
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

Successfully merging this pull request may close these issues.

4 participants