This is an addition to c1ab08c8ce where KT-26582 was fixed. The test
testClassLiteralArguments in JvmRuntimeDescriptorLoaderTestGenerated
failed before this change but went unnoticed because it wasn't run on CI
(see the previous commit)
Preface: for Groovy traits with fields, the Groovy compiler generates
synthetic "$Trait$FieldHelper" classes which posed several problems to
our class file reader, caused by the fact that the contents of the
InnerClasses attribute broke some assumptions about how names on the JVM
are formed and used.
For a trait named `A`, the Groovy compiler will additionally generate a
synthetic class file `A$Trait$FieldHelper` with the following in the
InnerClasses attribute:
InnerClasses:
public static #15= #2 of #14; //FieldHelper=class A$Trait$FieldHelper of class A
i.e. the simple name of the class is `FieldHelper`, the name of its
outer class is `A`, but the full internal name is `A$Trait$FieldHelper`,
which is surprising considering that the names are usually obtained by
separating the outer and inner names via the dollar sign.
Another detail is that in some usages of this synthetic class, the
InnerClasses attribute was missing at all. For example, if an empty
class `B` extends `A`, then there's no InnerClasses attribute in `B`'s
class file, which is surprising because we might decode the same name
differently depending on the class file we encounter it in.
In this change, we attempt to treat these synthetic classes as top-level
by refusing to read "invalid" InnerClasses attribute values (they are
not technically invalid because they still conform to JVMS), fixing the
problem of "unresolved supertypes" error which occurred when these
classes were used as supertypes in a class file in a dependency.
1) In ClassifierResolutionContext.mapInternalNameToClassId, do not use
the ad-hoc logic (copy-pasted from intellij-core) to determine class
id heuristically from the internal name. For $Trait$FieldHelper
classes this logic attempted to replace all dollar signs with dots,
which was semantically incorrect: dollars there were used as
synthetic characters, not as a separator between outer and inner
classes.
2) In isNotTopLevelClass (Other.kt), only consider "valid" InnerClasses
attribute values, where the full name of the class is obtained by
separating the outer name and the inner name with a dollar character.
This way, we'll be able to treat class files with invalid attribute
values as top-level and avoid breaking any other assumptions in the
class file loader.
3) In BinaryJavaClass.visitInnerClass, record all valid InnerClasses
attribute values present in the class file, not just those related to
the class in question itself. This is needed now because previously,
the removed heuristics (see p.1) transformed mentioned inner class
names to class ids correctly >99% of the time. Now that the
heuristics are gone, we'll use the information present in the class
file to map names correctly and predictably. According to JVMS, this
attribute should contain information about all inner classes
mentioned in the class file, and this is true at least for class
files produced by javac.
#KT-18592 Fixed
Only invariant array projections and non-null element types will be
supported soon (see KT-26568), so it makes no sense to store the
complete type in KClassValue. What we need is only the ClassId of the
class, and the number of times it's wrapped into kotlin/Array, which is
exactly what ClassLiteralValue represents.
This change helps in decoupling annotation values from
descriptors/types. The only constant value that depends on descriptors
is now AnnotationValue.
#KT-26582 Fixed
There is a trade-off between robustness of check and accuracy of the
diagnostic: the previous version, which works on generation, was too
fragile and lead to false-positives. Now we check on state machine
generation. However, since we do not have PSI for call, we can only
report diagnostic on whole suspend function or suspend lambda.
Additionally, the state machine is generated on crossinline suspend
lambdas regeneration and thus we do not have the PSI for the lambda as
well!
#KT-27130 Fixed
#KT-27258 Open
Declare AnalysisFlags in module 'frontend', and JvmAnalysisFlags in
module 'frontend.java', to avoid leaking Java-related logic to common
compiler code
* The members of Result are isSuccess, isFailure, exceptionOrNull, getOrNull
* The rest of API is implemented via inline-only extensions
* There are two internal functions to hide detailed mechanics of an internal
Result.Failure class: createFailure and throwOnFailure
* Result.toString is explicit: either Success(v) or Failure(x)
See KT-26538
Otherwise we're not trying to load annotations on the parameter of the
property setter in MemberDeserializer.loadProperty.
Note that after this commit, we could now also assume that if
getter/setter is default, it has no annotations, and thus use
Annotations.EMPTY for default getter/setter in loadProperty. However,
this would cause reflection to work incorrectly on classes compiled by
an older Kotlin compiler, so we'll still try to load annotations on
default accessors for an indefinite time.
#KT-25499 Fixed
Add PropertyDescriptor.backingField/delegateField to store annotations
on the field directly in an otherwise almost empty descriptor instance,
instead of storing them with use-sites in the corresponding property
descriptor. Instead of AnnotationWithTarget, create AnnotationDescriptor
instances in AnnotationSplitter. Change DescriptorRenderer to render
annotations on "related" declarations when needed, with the explicit
use-site target if applicable.
Most changes in diagnostic test data are related to the fact that
annotations which are known to have an incompatible use-site to the
declaration they're applied at (such as `@param:`-annotation on a
function), are now not loaded at all. It's fine because the code is
erroneous, so it doesn't really matter how do we load annotations with
invalid targets (some of this logic is also changed freely in subsequent
commits). Some changes are also explained by the fact that for example
an annotation on the property which is only applicable to FIELD is now
rendered with an explicit use-site target `@field:`, regardless of
whether it did have that use-site target syntactically or not.
Basically, after this change there's no point in calling
Annotations.getUseSiteTargetedAnnotations/getAllAnnotations anymore
because it's easier and more intuitive to just use Annotations of the
corresponding descriptor -- the backing / delegate field (introduced in
this commit) or the extension receiver / setter parameter (related
behavior was fixed in previous commits). Usages of
use-site-target-related methods will be refactored out in subsequent
commits
Only use the other setType that takes an instance of
ReceiverParameterDescriptor. This will make sure that call sites can
use correct receiver annotations
PSI-based implementation (accessible via
`-Xuse-old-class-files-reading`) loads parameter names from the
"MethodParameters" attribute if it's present, so our own implementation
should as well.
This metadata doesn't seem supported in the java.lang.model.element API
though, so SymbolBasedValueParameter (which is used in `-Xuse-javac`)
will continue to have incorrect behavior for now
#KT-25193 Fixed
Move parts of the logic to the only places where they're needed:
checking for public/final/val is only needed in
JvmFieldApplicabilityChecker, checking the proto flag is only needed in
reflection, checking the JvmField annotation presence is only needed in
backend
Previously, packages `java.lang` and `kotlin.jvm` were imported on JVM
by default on the same rights, causing problems when the same classifier
existed both in `java.lang` and `kotlin.jvm`. Since the only known case
of such conflict were type aliases to JVM classes, the corresponding
classes (expansions of those type aliases) were manually excluded from
default imports. This made the code in DefaultImportProvider complicated
and resulted in multiple problems, regarding both correctness and
performance (see 82364ad3e5, a9f2f5c7d0, dd3dbda719).
This change adds a new concept, a "low priority import", and treats
`java.lang` as such. Since these imports are now separated from the rest
of default imports in LazyImportScope via secondaryClassImportResolver,
conflicts between classifiers are handled naturally: the one from
`kotlin.jvm` always wins (unless the one from `java.lang` is imported
explicitly, of course). This approach is simpler, safer and does not
require any memory to cache anything.
Skip ResolveToJava.kt test for javac-based resolve; it now fails because
of a weird issue which I didn't have time to investigate (this is OK
because it's a corner case of an experimental functionality)
Alexander Udalov