Skip to content

Commit

Permalink
Remove common flags from type (parameter) flag model.
Browse files Browse the repository at this point in the history
  • Loading branch information
@rubenpieters @robinlefever
rubenpieters authored and robinlefever committed Nov 29, 2023
1 parent 22e974a commit 07afa7c
Show file tree
Hide file tree
Showing 10 changed files with 52 additions and 89 deletions.
2 changes: 0 additions & 2 deletions base/src/main/java/proguard/classfile/io/kotlin/KotlinMetadataWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1820,8 +1820,6 @@ private static int convertTypeParameterFlags(KotlinTypeParameterFlags flags)
{
Set<Flag> flagSet = new HashSet<>();

flagSet.addAll(convertCommonFlags(flags.common));

if (flags.isReified) flagSet.add(Flag.TypeParameter.IS_REIFIED);

return flagsOf(flagSet.toArray(new Flag[0]));
Expand Down
2 changes: 1 addition & 1 deletion base/src/main/java/proguard/classfile/kotlin/KotlinTypeMetadata.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ public boolean isStarProjection()
private static class KotlinStarProjectionMetadata
extends KotlinTypeMetadata
{
KotlinStarProjectionMetadata() { super(new KotlinTypeFlags(new KotlinCommonFlags())); }
KotlinStarProjectionMetadata() { super(new KotlinTypeFlags()); }

@Override
public boolean isStarProjection()
Expand Down
12 changes: 0 additions & 12 deletions base/src/main/java/proguard/classfile/kotlin/flags/KotlinTypeFlags.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,9 @@

/**
* Flags for Kotlin types.
*
* No valid common visibility or modality flags.
*
* hasAnnotation is valid.
*/
public class KotlinTypeFlags implements KotlinFlags
{

public final KotlinCommonFlags common;

/**
* Signifies that the corresponding type is marked as nullable, i.e. has a question mark at the end of its notation.
*/
Expand All @@ -44,9 +37,4 @@ public class KotlinTypeFlags implements KotlinFlags
* Signifies that the corresponding type is `suspend`.
*/
public boolean isSuspend;

public KotlinTypeFlags(KotlinCommonFlags common)
{
this.common = common;
}
}
11 changes: 0 additions & 11 deletions base/src/main/java/proguard/classfile/kotlin/flags/KotlinTypeParameterFlags.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,11 @@

/**
* Flags for Kotlin type parameters.
*
* No valid visibility or modality flags.
*
* hasAnnotation is valid.
*/
public class KotlinTypeParameterFlags implements KotlinFlags
{
public final KotlinCommonFlags common;

/**
* Signifies that the corresponding type parameter is `reified`.
*/
public boolean isReified;

public KotlinTypeParameterFlags(KotlinCommonFlags common)
{
this.common = common;
}
}
3 changes: 0 additions & 3 deletions base/src/main/java/proguard/classfile/kotlin/visitor/KotlinMetadataPrinter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,6 @@ public void visitAnyTypeParameter(Clazz clazz, KotlinTypeParameterMetadata kotli

pw.println(
typeParameterFlags(kotlinTypeParameterMetadata.flags) +
hasAnnotationsFlag(kotlinTypeParameterMetadata.flags.common) +
kotlinTypeParameterMetadata.name
);

Expand Down Expand Up @@ -912,8 +911,6 @@ else if (kotlinTypeMetadata.aliasName != null)
}
else
{
pw.print(hasAnnotationsFlag(kotlinTypeMetadata.flags.common));

if (kotlinTypeMetadata.className != null)
{
pw.print(externalClassName(kotlinTypeMetadata.className));
Expand Down
21 changes: 2 additions & 19 deletions base/src/main/java/proguard/classfile/util/kotlin/KotlinMetadataInitializer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -961,13 +961,6 @@ else if (kmType.getClassifier() instanceof KmClassifier.TypeAlias)
.map(KotlinAnnotationUtilKt::convertAnnotation)
.collect(Collectors.toList());


//PROBBUG if a value parameter or a type parameter has an annotation then
// the annotation will be stored there but the flag will be
// incorrectly set on this type. Sometimes the flag is not set
// when there are annotations, sometimes the flag is set but there are no annotations.
type.flags.common.hasAnnotations = !type.annotations.isEmpty();

return type;
}

Expand Down Expand Up @@ -1004,12 +997,6 @@ private static KotlinTypeParameterMetadata toKotlinTypeParameterMetadata(KmTypeP
.map(KotlinAnnotationUtilKt::convertAnnotation)
.collect(Collectors.toList());

//PROBBUG if a value parameter or a type parameter has an annotation then
// the annotation will be stored there but the flag will be
// incorrectly set on this type. Sometimes the flag is not set
// when there are annotations, sometimes the flag is set but there are no annotations.
kotlinTypeParameterMetadata.flags.common.hasAnnotations = !kotlinTypeParameterMetadata.annotations.isEmpty();

return kotlinTypeParameterMetadata;
}

Expand Down Expand Up @@ -1302,9 +1289,7 @@ private static KotlinFunctionFlags convertFunctionFlags(int kotlinFlags)

private static KotlinTypeFlags convertTypeFlags(int kotlinFlags)
{
KotlinTypeFlags flags = new KotlinTypeFlags(
convertCommonFlags(kotlinFlags)
);
KotlinTypeFlags flags = new KotlinTypeFlags();

flags.isNullable = Flag.Type.IS_NULLABLE.invoke(kotlinFlags);
flags.isSuspend = Flag.Type.IS_SUSPEND.invoke(kotlinFlags);
Expand All @@ -1316,9 +1301,7 @@ private static KotlinTypeFlags convertTypeFlags(int kotlinFlags)

private static KotlinTypeParameterFlags convertTypeParameterFlags(int kotlinFlags)
{
KotlinTypeParameterFlags flags = new KotlinTypeParameterFlags(
convertCommonFlags(kotlinFlags)
);
KotlinTypeParameterFlags flags = new KotlinTypeParameterFlags();

flags.isReified = Flag.TypeParameter.IS_REIFIED.invoke(kotlinFlags);

Expand Down
11 changes: 0 additions & 11 deletions base/src/test/kotlin/proguard/classfile/kotlin/flags/KotlinCommonFlagsTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,17 +183,6 @@ internal fun testHasAnnotation(prefix: String, clazz: Clazz, expected: Boolean,
}
}

test("$prefix: the hasAnnotations flag of the type param in $className should be $expected") {
verify(exactly = 1) {
kotlinTypeParamVisitor.visitAnyTypeParameter(
clazz,
withArg {
it.flags.common.hasAnnotations shouldBe expected
}
)
}
}

test("$prefix: the hasAnnotation flag of the value param in $className should be $expected") {
verify(exactly = 1) {
kotlinValueParamVisitor.visitAnyValueParameter(
Expand Down
22 changes: 0 additions & 22 deletions base/src/test/kotlin/proguard/classfile/kotlin/flags/KotlinTypeFlagsTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ class KotlinTypeFlagsTest : FreeSpec({
withArg {
it.flags.isNullable shouldBe false
it.flags.isSuspend shouldBe false

it.flags.common.hasAnnotations shouldBe false
}
)
}
Expand All @@ -73,8 +71,6 @@ class KotlinTypeFlagsTest : FreeSpec({
withArg {
it.flags.isNullable shouldBe false
it.flags.isSuspend shouldBe false

it.flags.common.hasAnnotations shouldBe false
}
)
}
Expand All @@ -93,8 +89,6 @@ class KotlinTypeFlagsTest : FreeSpec({
withArg {
it.flags.isNullable shouldBe true
it.flags.isSuspend shouldBe false

it.flags.common.hasAnnotations shouldBe false
}
)
}
Expand All @@ -110,8 +104,6 @@ class KotlinTypeFlagsTest : FreeSpec({
clazz,
withArg {
it.flags.isNullable shouldBe true

it.flags.common.hasAnnotations shouldBe false
}
)
}
Expand All @@ -130,8 +122,6 @@ class KotlinTypeFlagsTest : FreeSpec({
withArg {
it.flags.isNullable shouldBe false
it.flags.isSuspend shouldBe true

it.flags.common.hasAnnotations shouldBe false
}
)
}
Expand All @@ -148,8 +138,6 @@ class KotlinTypeFlagsTest : FreeSpec({
withArg {
it.flags.isNullable shouldBe false
it.flags.isSuspend shouldBe true

it.flags.common.hasAnnotations shouldBe false
}
)
}
Expand All @@ -169,8 +157,6 @@ class KotlinTypeFlagsTest : FreeSpec({
it.flags.isNullable shouldBe false
it.flags.isSuspend shouldBe false
it.flags.isDefinitelyNonNull shouldBe true

it.flags.common.hasAnnotations shouldBe false
}
)
}
Expand Down Expand Up @@ -214,8 +200,6 @@ class KotlinTypeFlagsTest : FreeSpec({
kotlinTypeMetadata.typeArguments?.get(0)?.flags?.isNullable shouldBe false
kotlinTypeMetadata.typeArguments?.get(0)?.flags?.isSuspend shouldBe false
kotlinTypeMetadata.typeArguments?.get(0)?.flags?.isDefinitelyNonNull shouldBe false

kotlinTypeMetadata.typeArguments?.get(0)?.flags?.common?.hasAnnotations shouldBe true
}
)
)
Expand All @@ -235,8 +219,6 @@ class KotlinTypeFlagsTest : FreeSpec({
kotlinTypeMetadata.typeArguments?.get(0)?.flags?.isNullable shouldBe false
kotlinTypeMetadata.typeArguments?.get(0)?.flags?.isSuspend shouldBe false
kotlinTypeMetadata.typeArguments?.get(0)?.flags?.isDefinitelyNonNull shouldBe false

kotlinTypeMetadata.typeArguments?.get(0)?.flags?.common?.hasAnnotations shouldBe true
}
)
)
Expand Down Expand Up @@ -267,8 +249,6 @@ class KotlinTypeFlagsTest : FreeSpec({
kotlinTypeMetadata.typeArguments?.get(0)?.flags?.isNullable shouldBe true
kotlinTypeMetadata.typeArguments?.get(0)?.flags?.isSuspend shouldBe false
kotlinTypeMetadata.typeArguments?.get(0)?.flags?.isDefinitelyNonNull shouldBe false

kotlinTypeMetadata.typeArguments?.get(0)?.flags?.common?.hasAnnotations shouldBe false
}
)
)
Expand All @@ -288,8 +268,6 @@ class KotlinTypeFlagsTest : FreeSpec({
kotlinTypeMetadata.typeArguments?.get(0)?.flags?.isNullable shouldBe true
kotlinTypeMetadata.typeArguments?.get(0)?.flags?.isSuspend shouldBe false
kotlinTypeMetadata.typeArguments?.get(0)?.flags?.isDefinitelyNonNull shouldBe false

kotlinTypeMetadata.typeArguments?.get(0)?.flags?.common?.hasAnnotations shouldBe false
}
)
)
Expand Down
55 changes: 47 additions & 8 deletions base/src/test/kotlin/proguard/classfile/kotlin/flags/KotlinTypeParameterFlagsTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ class KotlinTypeParameterFlagsTest : FreeSpec({
clazz,
withArg {
it.flags.isReified shouldBe false

it.flags.common.hasAnnotations shouldBe false
}
)
}
Expand All @@ -72,8 +70,6 @@ class KotlinTypeParameterFlagsTest : FreeSpec({
clazz,
withArg {
it.flags.isReified shouldBe false

it.flags.common.hasAnnotations shouldBe false
}
)
}
Expand All @@ -92,8 +88,6 @@ class KotlinTypeParameterFlagsTest : FreeSpec({
clazz,
withArg {
it.flags.isReified shouldBe true

it.flags.common.hasAnnotations shouldBe false
}
)
}
Expand All @@ -109,13 +103,58 @@ class KotlinTypeParameterFlagsTest : FreeSpec({
clazz,
withArg {
it.flags.isReified shouldBe true

it.flags.common.hasAnnotations shouldBe false
}
)
}
}
}

"Given a type parameter with annotation" - {
val clazz = ClassPoolBuilder.fromSource(
KotlinSource(
"Test.kt",
"""
@Target(AnnotationTarget.TYPE_PARAMETER)
annotation class Ann
inline fun <@Ann T> bar(): Int = 42
"""
)
).programClassPool.getClass("TestKt")

"Then the flags should be initialized correctly" {
clazz.accept(
ReferencedKotlinMetadataVisitor(
AllTypeParameterVisitor(
KotlinTypeParameterFilter(
{
true
},
{ _, kotlinTypeMetadata ->
kotlinTypeMetadata.flags.isReified shouldBe false
}
)
)
)
)
}

"Then the flags should be written and re-initialized correctly" {
clazz.accept(
ReWritingMetadataVisitor(
AllTypeParameterVisitor(
KotlinTypeParameterFilter(
{
true
},
{ _, kotlinTypeMetadata ->
kotlinTypeMetadata.flags.isReified shouldBe false
}
)
)
)
)
}
}
})

private fun createVisitor(typeName: String, typeVisitor: KotlinTypeParameterVisitor): KotlinMetadataVisitor =
Expand Down
2 changes: 2 additions & 0 deletions docs/md/releasenotes.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
- Enable fix previously behind system property: fix `TypedReferenceValue.generalize()` not setting `mayBeExtension` to true when generalizing to common parent type.
- Avoid printing `PartialEvaluator` messages when an `ExcessiveComplexityException` occurs.
- Fix incorrect writing of flags for type parameters with name annotations.
- Fix incorrect writing of flags for reified type parameters.
- Fix model for types and type parameters, removing the incorrect `HAS_ANNOTATION` common flag.

### Improved

Expand Down

0 comments on commit 07afa7c

Please sign in to comment.