A constant is a static final field with non-null value. In a previous
commit (0b09be7), we accidentally removed the *non-null value*
filter when looking for constants in the bytecode.
This commit re-adds that filter to make sure the detection is correct.
Test: Added KotlinOnlyClasspathChangesComputerTest.testDelegatedProperties
^KT-58986: Fixed
Both the new and old incremental compilation (IC) analysis rely on
Kotlin class metadata to detect a change.
However, Kotlin metadata currently doesn't contain info about
annotations (KT-57919), so the IC will not be able to detect a change
to them.
With this commit, we'll fix the new IC such that it can detect a change
to class annotations by not relying only on metadata.
We currently scope this fix to the new IC (cross-module analysis) first.
We'll fix this issue for within-module analysis later.
Performance: There seems to be no performance impact from this change.
Snapshotting the 400MB ideaIC-2022.1.4/app.jar takes 4.1s before and
after this change.
Test: Added ClasspathChangesComputerTest.testChangedAnnotations
^KT-58289: Fixed
A LookupSymbol should only refer to either a class, a class member, or a
package member.
When a LookupSymbol refers to a file facade (e.g.,
LookupSymbol(name="FooKt", scope="com.example")), it is redundant as it
doesn't impact the IC analysis to find files to recompile.
Previously, the new IC (ClasspathChangesComputer) would fail when
detecting that IncrementalJvmCache reported these redundant
LookupSymbols. With this change, the new IC will just ignore them.
Note: A better approach would be to fix IncrementalJvmCache to not
report these LookupSymbols, but it will require some significant
cleanup/refactoring work, so we can consider it later.
Test: New KotlinOnlyClasspathChangesComputerTest.testRenameFileFacade
^KT-55021 Fixed
If we detect a change in an inline function `foo` with @JvmName
`fooJvmName`, we have two options:
1. Report that function `foo` has changed
2. Report that method `fooJvmName` has changed
Similarly, if we detect a change in an inline property accessor with
JvmName `getFoo` of property `foo`, we have two options:
1. Report that property `foo` has changed
2. Report that property accessor `getFoo` has changed
The compiler is guaranteed to generate `LookupSymbol`s corresponding to
option 1 when referencing inline functions/property accessors, but it is
not guaranteed to generate `LookupSymbol`s corresponding to option 2.
(Currently the compiler seems to support option 2 for *inline*
functions/property accessors, but that may change.)
Therefore, we will choose option 1 as it is cleaner and safer.
^KT-54144 In progress
Small cleanup in IncrementalCompilerRunner
- Add comment for closing caches
- Rename providedChangedFiles to changedFiles
- Tiny clean up in `performWorkBeforeCompilation`
- Count directories to delete in debug logs
^KT-53015 In progress
Extract KotlinClassInfo to a separate class
to reduce the size of IncrementalJvmCache and prepare for the next
change.
^KT-54144 In progress
Ignore inline functions that are not found in the bytecode
^KT-54144 In progress
Add unit test for handling `@JvmName`s
Test: KotlinOnlyClasspathChangesComputerTest
#testFunctionsAndPropertyAccessorsWithJvmNames
^KT-54144 Fixed
Update unit tests for handling `@JvmName`s
In a previous commit, we made a behavior change for inline property
accessors: The existing behavior is that if the implementation of an
inline getter has changed, only usages of the getter will be impacted
but not usages of the setter (and vice versa).
After that previous commit, usages of *both* the getter and setter will
now be impacted (i.e., we might compile slightly more files). This is
because a change to either the getter or the setter will now be
considered a change to the property, which will help simplify our change
analysis.
This commit updates the relevant unit tests to reflect the new behavior.
Test: Updated Incremental*TestGenerated.PureKotlin#testInlinePropertyInClass
and Incremental*TestGenerated.PureKotlin#testInlinePropertyOnTopLevel
^KT-54144 Fixed
to reduce the size of IncrementalJvmCache and prepare for the next
change.
^KT-54144 In progress
Handle changes to inline functions/property accessors with `@JvmName`s
If we detect a change in an inline function `foo` with @JvmName
`fooJvmName`, we have two options:
1. Report that function `foo` has changed
2. Report that method `fooJvmName` has changed
Similarly, if we detect a change in an inline property accessor with
JvmName `getFoo` of property `foo`, we have two options:
1. Report that property `foo` has changed
2. Report that property accessor `getFoo` has changed
The compiler is guaranteed to generate `LookupSymbol`s corresponding to
option 1 when referencing inline functions/property accessors, but it is
not guaranteed to generate `LookupSymbol`s corresponding to option 2.
(Currently the compiler seems to support option 2 for *inline*
functions/property accessors, but that may change.)
Therefore, we will choose option 1 as it is cleaner and safer.
^KT-54144 In progress
Ignore inline functions that are not found in the bytecode
^KT-54144 In progress
Add unit test for handling `@JvmName`s
Test: KotlinOnlyClasspathChangesComputerTest
#testFunctionsAndPropertyAccessorsWithJvmNames
^KT-54144 Fixed
Small cleanup in IncrementalCompilerRunner
- Add comment for closing caches
- Rename providedChangedFiles to changedFiles
- Tiny clean up in `performWorkBeforeCompilation`
- Count directories to delete in debug logs
^KT-53015 In progress
Small cleanup in IncrementalCompilerRunner
- Add comment for closing caches
- Rename providedChangedFiles to changedFiles
- Tiny clean up in `performWorkBeforeCompilation`
- Count directories to delete in debug logs
^KT-53015 In progress
Problem: ClasspathChangesComputer (a core component of the new IC) uses
the existing inline function analysis from the old IC
(IncrementalJvmCache.InlineFunctionsMap). If an inline property accessor
has changed, the inline function analysis will report the name of the
property accessor, not the name of the property.
ClasspathChangesComputer doesn't see the property accessor's name in the
list of class/package members, so it will throw an exception.
Solution: There are 2 options:
1. If an inline property accessor has changed, the inline function
analysis can report the name of the property instead of the
property accessor.
2. ClasspathChangesComputer can include property accessors in the list
of class/package members.
In this commit, we will choose option 2 as it is simpler.
Test: New KotlinOnlyClasspathChangesComputerTest.testPropertyAccessors
Small cleanup - PLS SQUASH INTO PREVIOUS COMMIT
- Address review
- Fix failed tests
- Add some trivial changes
^KT-53871 Fixed
When computing impacted symbols of changed symbols, previously we
considered only the supertypes-inheritors type of impact, which is the
most common type. This commit adds the constants-in-companion-objects
type of impact to address KT-53266.
We've also cleaned up impact computation to make it easier to add new
types of impact in the future.
^KT-53266 In progress
To snapshot a Java class (+ its fields and methods), previously we used
Gson to serialize a class field/method to a string via reflection, and
hash that string.
We now use an ASM ClassWriter to write a placeholder class containing
the field/method of interest and hash the bytecode of that class.
One experiment showed that this new approach is ~10 times faster than
the previous approach (140s down to 16s when snapshotting 600 jars).
Test: Updated expectation files for JavaClassSnapshotterTest unit tests
+ Existing integration tests to prevent regression
^KT-52141 In Progress
1. CLASS_LEVEL: allows tracking whether a .class file has changed
without tracking what specific parts of the .class file (e.g.,
fields or methods) have changed.
2. CLASS_MEMBER_LEVEL: allows tracking not only whether a .class file
has changed but also what specific parts of the .class file (e.g.,
fields or methods) have changed.
The idea is that for better performance we will use CLASS_LEVEL for
classpath entries that are usually unchanged, and CLASS_MEMBER_LEVEL
for classpath entries that are frequently changed. We'll work out the
specifics in a following commit after some measurements.
Support running kotlinc on Windows in ClasspathSnapshotTestCommon
Also add tests for different Kotlin class kinds.
Add unit tests for CLASS_LEVEL snapshotting and diffing
Test: Updated ClasspathSnapshotterTest + ClasspathChangesComputerTest
Add ClasspathChangesComputerTest.testMixedClassSnapshotGranularities
to make it easier to add more tests in the next commits.
- Add unit tests for constants and inline functions
Also add tests for different kinds of Kotlin classes: CLASS,
FILE_FACADE, MULTIFILE_CLASS.
-Add unit test for nested classes
Also remove the existing integration test for nested classes to keep the
integration tests focused on the key scenarios while unit tests will
cover the corner cases.
Ignore inline functions that are private
Currently, we shrink classpath snapshots at 2 steps:
- Classpath diffing: Shrink the current classpath snapshot against
the previous lookup symbols
- Classpath snapshot saving: Shrink the current classpath snapshot
against the current lookup symbols
With this commit, the shrinking at the second step is now incremental.
The shrinking at the first step is still non-incremental.
Also visit a class file with ASM once to extract all information we need
in advance, instead of visiting the class file each time some piece of
info is needed.
Dmitriy Novozhilov