-
Notifications
You must be signed in to change notification settings - Fork 58
[XAT.Bytecode] Fix issue where Kotlin public types inside internal types were not being hidden. #827
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
[XAT.Bytecode] Fix issue where Kotlin public types inside internal types were not being hidden. #827
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,7 +30,7 @@ public static void Fixup (IList<ClassFile> classes) | |
var class_metadata = (metadata as KotlinClass); | ||
|
||
if (class_metadata != null) { | ||
FixupClassVisibility (c, class_metadata); | ||
FixupClassVisibility (c, class_metadata, classes); | ||
|
||
if (!c.AccessFlags.IsPubliclyVisible ()) | ||
continue; | ||
|
@@ -62,7 +62,7 @@ public static void Fixup (IList<ClassFile> classes) | |
} | ||
} | ||
|
||
static void FixupClassVisibility (ClassFile klass, KotlinClass metadata) | ||
static void FixupClassVisibility (ClassFile klass, KotlinClass metadata, IList<ClassFile> classes) | ||
{ | ||
// Hide class if it isn't Public/Protected | ||
if (klass.AccessFlags.IsPubliclyVisible () && !metadata.Visibility.IsPubliclyVisible ()) { | ||
|
@@ -83,15 +83,33 @@ static void FixupClassVisibility (ClassFile klass, KotlinClass metadata) | |
Log.Debug ($"Kotlin: Hiding internal class {klass.ThisClass.Name.Value}"); | ||
klass.AccessFlags = SetVisibility (klass.AccessFlags, ClassAccessFlags.Private); | ||
|
||
foreach (var ic in klass.InnerClasses) { | ||
Log.Debug ($"Kotlin: Hiding nested internal type {ic.InnerClass.Name.Value}"); | ||
ic.InnerClassAccessFlags = SetVisibility (ic.InnerClassAccessFlags, ClassAccessFlags.Private); | ||
} | ||
foreach (var ic in klass.InnerClasses) | ||
HideInternalInnerClass (ic, classes); | ||
|
||
return; | ||
} | ||
} | ||
|
||
static void HideInternalInnerClass (InnerClassInfo klass, IList<ClassFile> classes) | ||
{ | ||
var existing = klass.InnerClassAccessFlags; | ||
|
||
klass.InnerClassAccessFlags = SetVisibility (existing, ClassAccessFlags.Private); | ||
Log.Debug ($"Kotlin: Hiding nested internal type {klass.InnerClass.Name.Value}"); | ||
|
||
// Setting the inner class access flags above doesn't technically do anything, because we output | ||
// from the inner class's ClassFile, so we need to find that and set it there. | ||
var kf = classes.FirstOrDefault (c => c.ThisClass.Name.Value == klass.InnerClass.Name.Value); | ||
|
||
if (kf != null) { | ||
kf.AccessFlags = SetVisibility (kf.AccessFlags, ClassAccessFlags.Private); | ||
kf.InnerClass.InnerClassAccessFlags = SetVisibility (kf.InnerClass.InnerClassAccessFlags, ClassAccessFlags.Private); | ||
|
||
foreach (var inner_class in kf.InnerClasses.Where (c => c.OuterClass.Name.Value == kf.ThisClass.Name.Value)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this Wouldn't this more easily & efficiently be expressed as: foreach (var inner_class in kf.InnerClasses) {
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. …or does There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For some reason the That is, the |
||
HideInternalInnerClass (inner_class, classes); | ||
} | ||
} | ||
|
||
// Passing null for 'newVisibility' parameter means 'package-private' | ||
static ClassAccessFlags SetVisibility (ClassAccessFlags existing, ClassAccessFlags? newVisibility) | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
internal class InternalClassWithNestedInterface { | ||
public interface NestedInterface { | ||
public interface DoubleNestedInterface | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if
classes
should become aKeyedCollection<string, ClassFile>
? I'm just worried that ifclasses
gets "big",.FirstOrDefault()
may become a time sink.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potentially, though this should be a rarely used code path.