If there is an expression receiver, we should process constructors only
of inner classes. Constructors of nested classes can be called only
on classifier
^KT-65333 Fixed
If actual declaration is broken (missing explicit `actual` keyword),
expect declaration still makes sense.
This way, we allow refactorings on broken code
^KT-65191 fixed
- The cache guarantees deterministic cleanup on removal, so if the
reference has already been removed from the cache, there is no need to
clean it up again.
- Still, I don't want to guarantee that `SoftValueCleaner` is only
invoked once (see its documentation), as soft reference semantics
cannot be tested properly in our current test infrastructure.
^KT-61222
- Now it's checked that cleanup behaves as expected in a concurrent
setting for `put` and `remove`. Note that Lincheck never checks that a
value is actually cleaned up. Instead, it compares the result with the
single-threaded execution of a scenario. Hence, the non-concurrent
`CleanableSoftValueCacheTest` is crucial, as it separately ensures
correct deterministic cleanup in a single-threaded environment.
- We cannot test that operations returning a new value (such as `get`)
always return values which haven't been cleaned up yet, because the
value might be cleaned up by another thread between the return point
of the operation and cleanup checking.
^KT-62136
- This new version fixes problems with concurrent hash maps in model
checking tests and allows us to remove the correctness guarantee for
them.
^KT-62136
- The non-concurrent `CleanableSoftValueCacheTest` ensures that
deterministic cleanup behaves correctly. A Lincheck test can only
discover differences between the single-threaded and concurrent
executions of a test scenario, so it cannot find correctness issues
with deterministic cleanup on its own.
^KT-61222
- If we have some value `X` in the cache, and we put `X` a second time
into the same location, `X` shouldn't be cleaned up because it wasn't
actually removed from the cache.
- With this feature, it's necessary to implement `put` in terms of
`backingMap.compute`, because we need to compare the old and the new
value atomically, and based on that, decide whether to create a new
soft reference.
^KT-61222
- Lincheck tests the linearizability of a concurrent data structure,
which helps us verify that `CleanableSoftValueCache` works in
concurrent scenarios.
^KT-62136 fixed
- The previous implementation of `putIfAbsent` made two calls to the
`backingMap`: `putIfAbsent` and `replace`. This breaks atomicity at
least in theory. Implementing the major compute operations in terms of
`backingMap.compute` allows us to restrict the critical section to
this single atomic `ConcurrentHashMap` operation, which is easier to
reason about.
- Using `backingMap.compute` also improves the guarantees we can make in
respect to `computeIfAbsent`'s computation function `f`. We can now
guarantee that the function is called exactly once iff the `key` is
absent, because it is only ever invoked inside `backingMap.compute`,
which makes this guarantee itself.
- Remove `putIfAbsent`, which isn't currently used by
`LLFirSessionCache`. It can easily be implemented using
`computeIfAbsent` in the future.
^KT-61222
- These tests simply ensure that a resolve extension is disposed after
any kind of modification event is raised. It's not meant to test
complex scenarios, but rather to detect when resolve extension
disposal isn't invoked *at all*.
- I tried implementing a similar test for resolve extension disposal
after soft reference garbage collection, but the only way to force the
GC to collect soft references is to allocate memory until an
out-of-memory error occurs. That is bad for the test infrastructure,
because it might allocate A LOT of memory (depending on the max heap),
which is problematic for running tests locally. Also, our Kotlin tests
stop on an OOM error altogether (so additional configuration would be
required) and the heap is dumped into a 20GB file (on my machine),
which is again problematic for local test runs.
^KT-61222
- This allows tests to specify which modification event they want to
publish.
- The modification event won't be published automatically. Tests need to
invoke `publishModificationEventByDirective` at the right time.
^KT-56288
- This commit adds the `Disposable` interface to `KtResolveExtension`.
Resolve extensions are disposed after their associated session has
been invalidated, or reclaimed by the GC.
- Example use case: Opening a message bus connection with the lifetime
of the resolve extension.
^KT-61222 fixed
- This commit adds a `Disposable` to `LLFirSession` which lives as long
as the session. For example, session components can now open a message
bus connection that is automatically disposed after the session has
been invalidated or reclaimed.
- Because `LLFirSession`s should still benefit from soft reference-based
automatic reclamation by the GC, we have to use a soft value map with
cleanup in `LLFirSessionCache`. It ensures that the child disposables
are properly disposed even when the session is collected as a soft
value.
- The session cannot be the `Disposable` itself because a valid
disposable reference is needed to call `Disposer.dispose`. When a soft
reference is added to a reference queue (such as the reference queue
employed by the soft value map), its referent is already cleared.
Hence, it's not possible to watch a softly referenced `Disposable`
with a reference queue and also pass it to `Disposer.dispose`. This is
why the `LLFirSessionCleaner` contains a strong reference to the
`disposable`, because that will not keep the `LLFirSession` from being
reclaimed, while keeping the `disposable` alive slightly longer than
the session.
^KT-61222
- This cache implementation is useful for lifecycle management of cached
values when we also want to allow the GC to reclaim those values which
are unused. It ensures that the cleanup is (almost) definitely invoked
at some point after a value has been reclaimed by the GC, and also
invokes the same cleanup operation when the value is removed from the
cache conventionally. This unifies the approach to cleaning up such
elements.
- It would be possible to have the cache ONLY perform cleanup for
references reclaimed by the GC, but this would put the onus on the
consumer to perform the cleanup when an element is removed from the
cache conventionally. That comes with its own pitfalls, such as
requiring the user to handle `clear` correctly, or not forgetting
to perform cleanup with the old value returned by `put`.
- "Almost definitely" because the reference queue won't be processed if
the cache becomes unused at some point. However, this is not an issue
for the current intended use case (session invalidation and disposal)
and can be solved by clearing the cache before it becomes entirely
unused.
- The cache requires read and write actions for certain operations to
simplify the implementation of `clear`.
^KT-61222
This is an addition to d4278250e6. Apparently we still need to produce
a new fake override for each inherited static member, because otherwise
we would try to determine the most specific return type, maximum
visibility, etc, all of which makes no sense for static members.
#KT-66152 Fixed
This diagnostic is reported in rare situations when
StubTypeForBuilderInference is kept as a parameter type
of for loop or lambda. Before this commit, we had in K1
"Could not load module <error module>" from IrLinker instead.
Related to: KT-52757, KT-53109, KT-63841, KT-64066
#KT-53478 Fixed
- The module structure for these tests was set up backwards: We should
keep the test module kind of the main module flexible (as it is
configured by test configurators), but keep the module kinds of the
binary libraries fixed.
^KT-65960
^KT-64994
- The name wasn't specific enough.
- Remove default `emptyList()` argument from `CompiledLibraryProvider`
so that passing `dependencyBinaryRoots` isn't missed. (My changes
didn't include it in `KtLibraryBinaryModuleFactoryBase` yet, but it
was actually missed from `KtLibrarySourceModuleFactory` even in the
original commit.)
^KT-65960
^KT-64994
- The test infrastructure already requires test modules to be properly
ordered regarding dependencies, so a module A which is a dependency
for a module B must always be ordered before B. Hence, it makes no
sense to also order modules topologically.
- Reordering modules may cause the resulting `KtTestModule`s to be
out-of-order with the test modules, because the topological order
might be different from the test module order, even if the dependency
relationship already holds in the original order.
`KtTestModuleProjectStructure` requires the same ordering as test
modules to ensure the same order during iteration of `KtTestModule`s.
^KT-65960
^KT-64994
- Since binary libraries don't carry decompiled files anymore, the
static declaration provider doesn't index binary libraries in the
Standalone mode, where we don't use stub-based deserialization. This
led `LLSealedInheritorsProviderFactoryForTests` to fail in Standalone
mode for sealed classes from binary libraries, as this provider is now
populated from the declaration provider's index (see the previous
commit). Luckily, class-based deserialization sets
`sealedInheritorsAttr`, so we can simply use this attribute.
^KT-65960
- Now that binary libraries are decompiled to stubs instead of PSI
files, we cannot collect sealed inheritors from `KtFile`s anymore.
Since all `KtFile`s and binary library stubs are both indexed by the
declaration provider, we can collect inheritors from its index
instead.
- Invalidating all sessions at the end of `prepareSealedClassInheritors`
fixes some improper resolve phases in lazy resolution test data. While
the previous implementation requested an uncached resolve session, it
didn't account for sessions of dependencies still being cached.
^KT-65960
- Instead of indexing binary library declarations from decompiled PSI,
the static declaration provider now builds and indexes stubs. As noted
in KT-65960, this brings IDE mode tests much more in line with
decompiled stubs indexing in the IDE. It should allow us to catch
issues with the stub-based deserialized symbol provider outside of IDE
tests.
- In the Standalone mode, we can skip stub-indexing completely, as we
provide FIR symbols via class-based deserialization. This also extends
to shared binary roots.
^KT-65960 fixed
- `LibraryBinary` should not contain any decompiled files, as we want
FIR symbols in tests to be provided from indexed stubs or class files,
but definitely not from decompiled PSI. This brings `LibraryBinary`
much closer to the behavior of binary libraries in the IDE.
- Some tests may still require access to a decompiled file, for example
when trying to test `getOrBuildFir` for some `KtElement` coming from a
library. This commit introduces `LibraryBinaryDecompiled`, which
does contain decompiled files.
- We don't really need `LibraryBinary` as a main test module kind
anymore, since tests generally want to access some main `KtFile`.
Hence, test configurators for `LibraryBinary` have been turned into
configurators for `LibraryBinaryDecompiled`.
- An alternative would be decompiling files on demand, but this is not
currently feasible because the Standalone API doesn't reconcile stubs
with decompiled PSI, like the IDE does automatically. (For the same
declaration, the stub and the PSI will have a different identity.) As
long as there is no support for this, we'll have to rely on a separate
test module kind.
^KT-65960
- `TestModule.explicitTestModuleKind` requires a directive to be present
to get the `TestModuleKind`. But we still want to find out the test
module kind for test modules without a directive. Hence, we have to
add this property to `KtTestModule` during its construction.
^KT-65960
- `KtModuleWithFiles` isn't actually used in a production Standalone API
context, but it was exposed via `analysis-api-standalone-base`. In
current production usages, the project structure is built with the
module builder DSL.
- Hence, `KtModuleWithFiles` is only relevant for tests. This commit
moves `KtModuleWithFiles` to the Analysis API test framework and
renames it to `KtTestModule`. This removes any risk that an outside
user could start using `KtModuleWithFiles` and completely uncouples
the test project structure from production APIs.
- In addition, we can add the `TestModule` to `KtTestModule`, allowing
tests to quickly access the original test module, for example to check
the test module kind.
- The commit also removes the data class status of `KtTestModule` and
`KtTestModuleProjectStructure` to avoid issues with destructuring when
properties are added or removed.
^KT-65960
- The configuration allows the test infrastructure to decide whether to
index binary libraries to stubs (when stub-based deserialized symbol
providers are used) or to skip indexing (when class file-based
deserialization is used).
- The information is needed in `AnalysisApiBaseTestServiceRegistrar`,
where the `KotlinStaticDeclarationProviderFactory` is created. This
service registrar shouldn't access `LLFirLibrarySymbolProviderFactory`
and so checking the library symbol provider factory wasn't an option.
- Another alternative was adding a property to
`AnalysisApiTestConfigurator`. However, this then requires passing the
property to `AnalysisApiBaseTestServiceRegistrar`, because it doesn't
have access to the configurator out of the box. This however goes
against the design of our service registrars, which generally only
access test services. So adding a test service seemed like the best
solution.
^KT-65960
When a symbol X from a binary library A uses another symbol Y from
another binary library B, `LLFirDependenciesSymbolProvider` cannot
resolve Y for X. This is because the existing
LLFirAbstractSessionFactory passes symbol providers for only builtin
libraries to `LLFirDependenciesSymbolProvider` even when the session has
dependencies. As a result, when `LLFirDependenciesSymbolProvider`
searches a symbol, it can find only symbols from the builtin libraries,
but it cannot find a symbol from libraries other than builtin libraries.
This happens only for the binary libraries.
^KT-65240 Fixed
Usually we create synthetic property in java class if there is a property
from the base kotlin class and getter/setter with a corresponding name
in the declared scope or one of supertype scopes. But there is a case,
where the same supertype contains both property and getter:
```
// FILE: Base.kt
open class Base {
open val b = "O"
@JvmName("getBJava")
fun getB() : String = "K"
}
// FILE: Derived.java
public class Derived extends Base {}
```
In this case we shouldn't create synthetic property, because `getB()`
function is invisible for `Derived` class
^KT-66020 Fixed
- The fix uses relevant logic from `getNonLocalContainingDeclaration`.
- The annotation entry case is covered by `KtCallElement` because
`KtAnnotationEntry` is a subtype of it.
- KT-66038 is fixed by this because the static declaration provider only
indexes non-local classes.
^KT-66038 fixed
In K1, code fragment analysis was completely invalidated on any PSI
change. Because imports are not a part of the PSI tree of
'KtCodeFragment's, a colon tremble happened on 'addImportsFromString()'.
In K2, changes inside code fragments are always considered in-body
modifications. So, even with the colon trembling, the 'FirFile',
together with its 'FirImport's was not recreated.
^KT-65600 Fixed
Dmitriy Novozhilov