Fix for KT-1310 Incorrect hint #2
Merged
Conversation
This file contains 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
abreslav
added a commit
that referenced
this pull request
Feb 29, 2012
Fix for KT-1310 Incorrect hint (by Sergey Lukjanov)
geevee
pushed a commit
that referenced
this pull request
Jun 9, 2012
yanex
added a commit
that referenced
this pull request
Feb 18, 2015
README corrections: Understanding Koan
ilya-g
added a commit
that referenced
this pull request
Jun 29, 2016
ligee
added a commit
that referenced
this pull request
Oct 2, 2016
ligee
added a commit
that referenced
this pull request
Oct 11, 2016
ligee
added a commit
that referenced
this pull request
Oct 11, 2016
ligee
added a commit
that referenced
this pull request
Oct 12, 2016
shiraji
pushed a commit
to shiraji/kotlin
that referenced
this pull request
Oct 14, 2016
mglukhikh
pushed a commit
that referenced
this pull request
Jun 26, 2018
Propagated variables classes have been made due to switch-case problem. Since trivial vals can be equal to the result of simple operation(i.e plus, minus), we have to find all pseudovalues needed for this operation, which can be not as effective as we want
udalov
added a commit
that referenced
this pull request
Oct 22, 2018
Preface: for Groovy traits with fields which are extended by classes,
the Groovy compiler generates synthetic "$Trait$FieldHelper" classes
which posed several problems to our class file reader, caused by the
fact the contents of the InnerClasses attribute broke some assumptions
about how names on the JVM are formed.
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 occrurred 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
udalov
added a commit
that referenced
this pull request
Oct 24, 2018
Preface: for Groovy traits with fields which are extended by classes,
the Groovy compiler generates synthetic "$Trait$FieldHelper" classes
which posed several problems to our class file reader, caused by the
fact the contents of the InnerClasses attribute broke some assumptions
about how names on the JVM are formed.
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 occrurred 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
udalov
added a commit
that referenced
this pull request
Oct 25, 2018
Preface: for Groovy traits with fields which are extended by classes,
the Groovy compiler generates synthetic "$Trait$FieldHelper" classes
which posed several problems to our class file reader, caused by the
fact the contents of the InnerClasses attribute broke some assumptions
about how names on the JVM are formed.
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 occrurred 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
udalov
added a commit
that referenced
this pull request
Oct 25, 2018
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 occrurred 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
udalov
added a commit
that referenced
this pull request
Oct 25, 2018
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
ausatiy
pushed a commit
that referenced
this pull request
Nov 1, 2018
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
kishmakov
added a commit
that referenced
this pull request
Jan 24, 2019
Alefas
pushed a commit
that referenced
this pull request
Mar 21, 2019
anastasiiaSpaseeva-zz
pushed a commit
that referenced
this pull request
Nov 28, 2019
anastasiiaSpaseeva-zz
pushed a commit
that referenced
this pull request
Nov 29, 2019
anastasiiaSpaseeva-zz
pushed a commit
that referenced
this pull request
Dec 2, 2019
ilgonmic
added a commit
that referenced
this pull request
Jan 21, 2020
lunakoly
added a commit
that referenced
this pull request
Feb 21, 2024
And refactor another related test to explain the desired behavior. ^KT-65972
lunakoly
added a commit
that referenced
this pull request
Feb 21, 2024
KotlinBuild
pushed a commit
that referenced
this pull request
Feb 21, 2024
And refactor another related test to explain the desired behavior. ^KT-65972
KotlinBuild
pushed a commit
that referenced
this pull request
Feb 21, 2024
KotlinBuild
pushed a commit
that referenced
this pull request
Mar 11, 2024
The Fir2Ir has two steps in terms of the annotation setup: Step #1. Creating `IrMutableAnnotationContainer` Step #2. Filling `IrMutableAnnotationContainer` with annotations When the `IrMutableAnnotationContainer` is out of the source module, `Fir2IrCallableDeclarationsGenerator` sets annotations for a `IrMutableAnnotationContainer` when creating its IR object. For example, when `foo()` defined in `B.kt` is called in file `A.kt`, and `A.kt` is a part of the source module while `B.kt` is a part of library, `Fir2IrCallableDeclarationsGenerator` sets annotations of `IrFunction` for `foo()`. On the other hand, if `foo()` is a part of the source module, `Fir2IrCallableDeclarationsGenerator` does not set annotations for it when creating `IrMutableAnnotationContainer` (Step #1). Later, when it fills contents of `IrMutableAnnotationContainer`, it conducts Step #2. However, if `foo()` is a part of the source module, but it is not in a compile target file, filling contents of `foo()` does not happen (because it is not a compile target). As a result, set up annotations for `foo()` is not done by Fir2Ir. This missing annotation setup causes a serious bug for Android Compose app. This commit updates code to decide whether we have to set up the annotations of `IrMutableAnnotationContainer` in Step #1 or not. We have to check not only if it is a part of the source code but also if it is a part of compile targets or not.
KotlinBuild
pushed a commit
that referenced
this pull request
Mar 12, 2024
The Fir2Ir has two steps in terms of the annotation setup: Step #1. Creating `IrMutableAnnotationContainer` Step #2. Filling `IrMutableAnnotationContainer` with annotations When the `IrMutableAnnotationContainer` is out of the source module, `Fir2IrCallableDeclarationsGenerator` sets annotations for a `IrMutableAnnotationContainer` when creating its IR object. For example, when `foo()` defined in `B.kt` is called in file `A.kt`, and `A.kt` is a part of the source module while `B.kt` is a part of library, `Fir2IrCallableDeclarationsGenerator` sets annotations of `IrFunction` for `foo()`. On the other hand, if `foo()` is a part of the source module, `Fir2IrCallableDeclarationsGenerator` does not set annotations for it when creating `IrMutableAnnotationContainer` (Step #1). Later, when it fills contents of `IrMutableAnnotationContainer`, it conducts Step #2. However, if `foo()` is a part of the source module, but it is not in a compile target file, filling contents of `foo()` does not happen (because it is not a compile target). As a result, set up annotations for `foo()` is not done by Fir2Ir. This missing annotation setup causes a serious bug for Android Compose app. This commit updates code to decide whether we have to set up the annotations of `IrMutableAnnotationContainer` in Step #1 or not. We have to check not only if it is a part of the source code but also if it is a part of compile targets or not. ^KT-66532 Fixed
KotlinBuild
pushed a commit
that referenced
this pull request
Mar 14, 2024
The Fir2Ir has two steps in terms of the annotation setup: Step #1. Creating `IrMutableAnnotationContainer` Step #2. Filling `IrMutableAnnotationContainer` with annotations When the `IrMutableAnnotationContainer` is out of the source module, `Fir2IrCallableDeclarationsGenerator` sets annotations for a `IrMutableAnnotationContainer` when creating its IR object. For example, when `foo()` defined in `B.kt` is called in file `A.kt`, and `A.kt` is a part of the source module while `B.kt` is a part of library, `Fir2IrCallableDeclarationsGenerator` sets annotations of `IrFunction` for `foo()`. On the other hand, if `foo()` is a part of the source module, `Fir2IrCallableDeclarationsGenerator` does not set annotations for it when creating `IrMutableAnnotationContainer` (Step #1). Later, when it fills contents of `IrMutableAnnotationContainer`, it conducts Step #2. However, if `foo()` is a part of the source module, but it is not in a compile target file, filling contents of `foo()` does not happen (because it is not a compile target). As a result, set up annotations for `foo()` is not done by Fir2Ir. This missing annotation setup causes a serious bug for Android Compose app. This commit updates code to decide whether we have to set up the annotations of `IrMutableAnnotationContainer` in Step #1 or not. We have to check not only if it is a part of the source code but also if it is a part of compile targets or not. ^KT-66532 Fixed
KotlinBuild
pushed a commit
that referenced
this pull request
Mar 20, 2024
The Fir2Ir has two steps in terms of the annotation setup: Step #1. Creating `IrMutableAnnotationContainer` Step #2. Filling `IrMutableAnnotationContainer` with annotations When the `IrMutableAnnotationContainer` is out of the source module, `Fir2IrCallableDeclarationsGenerator` sets annotations for a `IrMutableAnnotationContainer` when creating its IR object. For example, when `foo()` defined in `B.kt` is called in file `A.kt`, and `A.kt` is a part of the source module while `B.kt` is a part of library, `Fir2IrCallableDeclarationsGenerator` sets annotations of `IrFunction` for `foo()`. On the other hand, if `foo()` is a part of the source module, `Fir2IrCallableDeclarationsGenerator` does not set annotations for it when creating `IrMutableAnnotationContainer` (Step #1). Later, when it fills contents of `IrMutableAnnotationContainer`, it conducts Step #2. However, if `foo()` is a part of the source module, but it is not in a compile target file, filling contents of `foo()` does not happen (because it is not a compile target). As a result, set up annotations for `foo()` is not done by Fir2Ir. This missing annotation setup causes a serious bug for Android Compose app. This commit updates code to decide whether we have to set up the annotations of `IrMutableAnnotationContainer` in Step #1 or not. We have to check not only if it is a part of the source code but also if it is a part of compile targets or not. ^KT-66532 Fixed
demiurg906
pushed a commit
that referenced
this pull request
Mar 22, 2024
The Fir2Ir has two steps in terms of the annotation setup: Step #1. Creating `IrMutableAnnotationContainer` Step #2. Filling `IrMutableAnnotationContainer` with annotations When the `IrMutableAnnotationContainer` is out of the source module, `Fir2IrCallableDeclarationsGenerator` sets annotations for a `IrMutableAnnotationContainer` when creating its IR object. For example, when `foo()` defined in `B.kt` is called in file `A.kt`, and `A.kt` is a part of the source module while `B.kt` is a part of library, `Fir2IrCallableDeclarationsGenerator` sets annotations of `IrFunction` for `foo()`. On the other hand, if `foo()` is a part of the source module, `Fir2IrCallableDeclarationsGenerator` does not set annotations for it when creating `IrMutableAnnotationContainer` (Step #1). Later, when it fills contents of `IrMutableAnnotationContainer`, it conducts Step #2. However, if `foo()` is a part of the source module, but it is not in a compile target file, filling contents of `foo()` does not happen (because it is not a compile target). As a result, set up annotations for `foo()` is not done by Fir2Ir. This missing annotation setup causes a serious bug for Android Compose app. This commit updates code to decide whether we have to set up the annotations of `IrMutableAnnotationContainer` in Step #1 or not. We have to check not only if it is a part of the source code but also if it is a part of compile targets or not. ^KT-66532 Fixed
yanex
pushed a commit
that referenced
this pull request
Mar 25, 2024
The Fir2Ir has two steps in terms of the annotation setup: Step #1. Creating `IrMutableAnnotationContainer` Step #2. Filling `IrMutableAnnotationContainer` with annotations When the `IrMutableAnnotationContainer` is out of the source module, `Fir2IrCallableDeclarationsGenerator` sets annotations for a `IrMutableAnnotationContainer` when creating its IR object. For example, when `foo()` defined in `B.kt` is called in file `A.kt`, and `A.kt` is a part of the source module while `B.kt` is a part of library, `Fir2IrCallableDeclarationsGenerator` sets annotations of `IrFunction` for `foo()`. On the other hand, if `foo()` is a part of the source module, `Fir2IrCallableDeclarationsGenerator` does not set annotations for it when creating `IrMutableAnnotationContainer` (Step #1). Later, when it fills contents of `IrMutableAnnotationContainer`, it conducts Step #2. However, if `foo()` is a part of the source module, but it is not in a compile target file, filling contents of `foo()` does not happen (because it is not a compile target). As a result, set up annotations for `foo()` is not done by Fir2Ir. This missing annotation setup causes a serious bug for Android Compose app. This commit updates code to decide whether we have to set up the annotations of `IrMutableAnnotationContainer` in Step #1 or not. We have to check not only if it is a part of the source code but also if it is a part of compile targets or not. ^KT-66532 Fixed
KotlinBuild
pushed a commit
that referenced
this pull request
Mar 26, 2024
The Fir2Ir has two steps in terms of the annotation setup: Step #1. Creating `IrMutableAnnotationContainer` Step #2. Filling `IrMutableAnnotationContainer` with annotations When the `IrMutableAnnotationContainer` is out of the source module, `Fir2IrCallableDeclarationsGenerator` sets annotations for a `IrMutableAnnotationContainer` when creating its IR object. For example, when `foo()` defined in `B.kt` is called in file `A.kt`, and `A.kt` is a part of the source module while `B.kt` is a part of library, `Fir2IrCallableDeclarationsGenerator` sets annotations of `IrFunction` for `foo()`. On the other hand, if `foo()` is a part of the source module, `Fir2IrCallableDeclarationsGenerator` does not set annotations for it when creating `IrMutableAnnotationContainer` (Step #1). Later, when it fills contents of `IrMutableAnnotationContainer`, it conducts Step #2. However, if `foo()` is a part of the source module, but it is not in a compile target file, filling contents of `foo()` does not happen (because it is not a compile target). As a result, set up annotations for `foo()` is not done by Fir2Ir. This missing annotation setup causes a serious bug for Android Compose app. This commit updates code to decide whether we have to set up the annotations of `IrMutableAnnotationContainer` in Step #1 or not. We have to check not only if it is a part of the source code but also if it is a part of compile targets or not. ^KT-66532 Fixed
KotlinBuild
pushed a commit
that referenced
this pull request
Apr 15, 2024
The Fir2Ir has two steps in terms of the annotation setup: Step #1. Creating `IrMutableAnnotationContainer` Step #2. Filling `IrMutableAnnotationContainer` with annotations When the `IrMutableAnnotationContainer` is out of the source module, `Fir2IrCallableDeclarationsGenerator` sets annotations for a `IrMutableAnnotationContainer` when creating its IR object. For example, when `foo()` defined in `B.kt` is called in file `A.kt`, and `A.kt` is a part of the source module while `B.kt` is a part of library, `Fir2IrCallableDeclarationsGenerator` sets annotations of `IrFunction` for `foo()`. On the other hand, if `foo()` is a part of the source module, `Fir2IrCallableDeclarationsGenerator` does not set annotations for it when creating `IrMutableAnnotationContainer` (Step #1). Later, when it fills contents of `IrMutableAnnotationContainer`, it conducts Step #2. However, if `foo()` is a part of the source module, but it is not in a compile target file, filling contents of `foo()` does not happen (because it is not a compile target). As a result, set up annotations for `foo()` is not done by Fir2Ir. This missing annotation setup causes a serious bug for Android Compose app. This commit updates code to decide whether we have to set up the annotations of `IrMutableAnnotationContainer` in Step #1 or not. We have to check not only if it is a part of the source code but also if it is a part of compile targets or not. ^KT-66532 Fixed
dsavvinov
added a commit
that referenced
this pull request
Apr 24, 2024
…ist/ Interop klibs are built into two steps: 1. Compile interop klibs in kotlin-native/platformLibs/build/konan/libs This is done in KonanInteropTasks with names like: 'compileKonanAndroid_arm64-mediaAndroid_arm64' 2. Move them to kotlin-native/dist/klib/platform This is done in regular Sync-tasks with names like: 'android_arm64-media' Some interop klibs depend on other interop klibs (e.g. android_arm64/media library depends on android_arm64/posix). This commit changes how this dependency is declared; Before: - KonanInteropTasks declared dependencies on raw files in kotlin-native/dist/klib/platform. Example: 'compileKonanAndroid_arm64-mediaAndroid_arm64' declared a dependency on 'kotlin-native/dist/klib/platform/android_arm64/o.j.k.n.p.posix'-dir - KonanInteropTasks declared dependencies on Sync-tasks, i.e. tasks from #1 declare dependencies on tasks from #2. Example: 'compileKonanAndroid_arm64-mediaAndroid_arm64' declared a dependency on 'android_arm64-posix' After: - KonanInteropTasks declare dependencies on raw files in kotlin-native/platformLibs/build/konan/libs. Example: 'compileKonanAndroid_arm64-mediaAndroid_arm64' declares a dependency on 'kotlin-native/platformLibs/build/konan/libs/android_arm64/o.j.k.n.p.posix' - KonanInteropTasks declare dependencies on other KonanInteropTasks. Example: 'compileKonanAndroid_arm64-mediaAndroid_arm64' declares a dependency on 'compileKonanAndroid_arm64-posixAndroid_arm64' This improves incapsulation and makes it less likely for inputs of these tasks to overlap with outputs of other tasks that write into dist/ (and overlapping outputs lead to error in Gradle)
dsavvinov
added a commit
that referenced
this pull request
May 13, 2024
…ist/ Interop klibs are built into two steps: 1. Compile interop klibs in kotlin-native/platformLibs/build/konan/libs This is done in KonanInteropTasks with names like: 'compileKonanAndroid_arm64-mediaAndroid_arm64' 2. Move them to kotlin-native/dist/klib/platform This is done in regular Sync-tasks with names like: 'android_arm64-media' Some interop klibs depend on other interop klibs (e.g. android_arm64/media library depends on android_arm64/posix). This commit changes how this dependency is declared; Before: - KonanInteropTasks declared dependencies on raw files in kotlin-native/dist/klib/platform. Example: 'compileKonanAndroid_arm64-mediaAndroid_arm64' declared a dependency on 'kotlin-native/dist/klib/platform/android_arm64/o.j.k.n.p.posix'-dir - KonanInteropTasks declared dependencies on Sync-tasks, i.e. tasks from #1 declare dependencies on tasks from #2. Example: 'compileKonanAndroid_arm64-mediaAndroid_arm64' declared a dependency on 'android_arm64-posix' After: - KonanInteropTasks declare dependencies on raw files in kotlin-native/platformLibs/build/konan/libs. Example: 'compileKonanAndroid_arm64-mediaAndroid_arm64' declares a dependency on 'kotlin-native/platformLibs/build/konan/libs/android_arm64/o.j.k.n.p.posix' - KonanInteropTasks declare dependencies on other KonanInteropTasks. Example: 'compileKonanAndroid_arm64-mediaAndroid_arm64' declares a dependency on 'compileKonanAndroid_arm64-posixAndroid_arm64' This improves incapsulation and makes it less likely for inputs of these tasks to overlap with outputs of other tasks that write into dist/ (and overlapping outputs lead to error in Gradle)
KotlinBuild
pushed a commit
that referenced
this pull request
May 14, 2024
…ist/ Interop klibs are built into two steps: 1. Compile interop klibs in kotlin-native/platformLibs/build/konan/libs This is done in KonanInteropTasks with names like: 'compileKonanAndroid_arm64-mediaAndroid_arm64' 2. Move them to kotlin-native/dist/klib/platform This is done in regular Sync-tasks with names like: 'android_arm64-media' Some interop klibs depend on other interop klibs (e.g. android_arm64/media library depends on android_arm64/posix). This commit changes how this dependency is declared; Before: - KonanInteropTasks declared dependencies on raw files in kotlin-native/dist/klib/platform. Example: 'compileKonanAndroid_arm64-mediaAndroid_arm64' declared a dependency on 'kotlin-native/dist/klib/platform/android_arm64/o.j.k.n.p.posix'-dir - KonanInteropTasks declared dependencies on Sync-tasks, i.e. tasks from #1 declare dependencies on tasks from #2. Example: 'compileKonanAndroid_arm64-mediaAndroid_arm64' declared a dependency on 'android_arm64-posix' After: - KonanInteropTasks declare dependencies on raw files in kotlin-native/platformLibs/build/konan/libs. Example: 'compileKonanAndroid_arm64-mediaAndroid_arm64' declares a dependency on 'kotlin-native/platformLibs/build/konan/libs/android_arm64/o.j.k.n.p.posix' - KonanInteropTasks declare dependencies on other KonanInteropTasks. Example: 'compileKonanAndroid_arm64-mediaAndroid_arm64' declares a dependency on 'compileKonanAndroid_arm64-posixAndroid_arm64' This improves incapsulation and makes it less likely for inputs of these tasks to overlap with outputs of other tasks that write into dist/ (and overlapping outputs lead to error in Gradle)
ilgonmic
added a commit
that referenced
this pull request
May 24, 2024
ilgonmic
added a commit
that referenced
this pull request
May 27, 2024
ilgonmic
added a commit
that referenced
this pull request
Nov 7, 2024
ilgonmic
added a commit
that referenced
this pull request
Nov 7, 2024
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.
Here is a small fix for '=>' hint in when statement instead of '->'. In fact, it is replacement of one to other. Additionally, I fix corresponding test data.