Fix emission of annotations #1763
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Looks like this unmasked an issue with the inline body annotations, investigating... |
|
Backend-wise LGTM |
`isRuntimeVisible` and `retentionPolicyOf` had two different ways to get the retention policy of an annotation and they were both wrong. Fix retentionPolicyOf` and use it in `isRuntimeVisible`
Previously we replaced them by ConcreteAnnotation so they became regular annotations and could be emitted in some cases. They need to keep being BodyAnnotation.
smarter
force-pushed
the
fix/annotations
branch
from
9b52ca5 to
3d0accd
Compare
smarter
changed the title
|
This still isn't enough to fix #1741, turns out sbt uses the incremental compilation analysis to know which methods have special annotations and need to be run with a test interface, but I never implemented support for annotations in incremental compilation: https://github.com/lampepfl/dotty/blob/b9350f40990ce07ba7614a0448a98abd7075abe8/compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala#L501, so I know what I need to do next! |
smarter
force-pushed
the
fix/annotations
branch
from
5979c20 to
2dc65a9
Compare
|
Alright, #1741 is fixed by the latest commit, and I added a test to test that the tests are testing something :) |
This is necessary for correct incremental recompilation but is also used by sbt to find tests to run (for junit they should be annotated @org.junit.Test). I added an sbt scripted test to verify that JUnit now works, to run it: $ sbt > scripted discovery/test-discovery
smarter
force-pushed
the
fix/annotations
branch
from
2dc65a9 to
9411539
Compare
|
Well done @smarter 👍 ! |
|
Ping @DarkDimius , would you mind review the remaining commits? |
|
LGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Review by @DarkDimius