Add shrinker-test examples with never-referenced constructors #2478
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
R8 can do some optimizations that assume values of types whose constructors are never referenced are `null`. (A bit confused why the rules here are sufficient to avoid that optimization in the case of a class whose sole constructor has arguments and is unreferenced, since the keep rule only refers to `<init>()` instead of `<init>(...)`. Does R8 have special behavior for `<init>()` where attempting to keep that makes it assume a class is constructable even if it doesn't have a no-args constructor?) Also rename some test classes/names to refer to `NoArgs` instead of `Default` constructors. The examples here care about whether a no-argument constructor is present. While a no-argument constructor (explicitly defined or not) is used by default in some circumstances, the Java documentation consistently uses "default constructor" to refer only to constructors whose implementation is implicitly provided by the complier (all default constructors are no-argument constructors, but not vice versa).
Marcono1234
reviewed
Aug 26, 2023
There was a problem hiding this comment.
Thanks a lot! The changes look good, just a few small things (see comments below).
R8 can do some optimizations that assume values of types whose constructors are never referenced are
null
I think by accident I triggered this for ProGuard while initially creating the shrinker-tests, respectively I triggered the cryptic NullPointerException this is causing (with JVM's helpful NullPointerException messages it says something like 'Cannot throw exception because "null" is null'). Though I am not sure if I still have the code to cause this, and if it would still cause this with the latest gson.pro file.
A bit confused why the rules here are sufficient to avoid that optimization...
I am not sure either, but I am also not very familiar with the details of R8.
Also rename some test classes/names to refer to
NoArgsinstead ofDefaultconstructors
Good point! You are right, the JLS seems to use the term "default constructor" only for the implicit one. I was mixing the terms a bit inconsistently.
shrinker-test/src/main/java/com/example/ClassWithNoArgsConstructor.java
Outdated
Show resolved
Hide resolved
Marcono1234
reviewed
Aug 26, 2023
shrinker-test/src/main/java/com/example/NoSerializedNameMain.java
Outdated
Show resolved
Hide resolved
Marcono1234
reviewed
Aug 26, 2023
shrinker-test/src/main/java/com/example/NoSerializedNameMain.java
Outdated
Show resolved
Hide resolved
…ctor.java Accept review suggestion Co-authored-by: Marcono1234 <[email protected]>
Accept review suggestion Co-authored-by: Marcono1234 <[email protected]>
Marcono1234
reviewed
Sep 4, 2023
shrinker-test/src/main/java/com/example/NoSerializedNameMain.java
Outdated
Show resolved
Hide resolved
Marcono1234
approved these changes
Sep 4, 2023
eamonnmcmanus
approved these changes
Sep 5, 2023
Marcono1234
added
the
proguard-r8
tibor-universe
pushed a commit
to getuniverse/gson
that referenced
this pull request
Sep 14, 2024
…#2478) * Add shrinker-test examples with never-referenced constructors R8 can do some optimizations that assume values of types whose constructors are never referenced are `null`. (A bit confused why the rules here are sufficient to avoid that optimization in the case of a class whose sole constructor has arguments and is unreferenced, since the keep rule only refers to `<init>()` instead of `<init>(...)`. Does R8 have special behavior for `<init>()` where attempting to keep that makes it assume a class is constructable even if it doesn't have a no-args constructor?) Also rename some test classes/names to refer to `NoArgs` instead of `Default` constructors. The examples here care about whether a no-argument constructor is present. While a no-argument constructor (explicitly defined or not) is used by default in some circumstances, the Java documentation consistently uses "default constructor" to refer only to constructors whose implementation is implicitly provided by the complier (all default constructors are no-argument constructors, but not vice versa). * Update shrinker-test/src/main/java/com/example/ClassWithNoArgsConstructor.java Accept review suggestion Co-authored-by: Marcono1234 <[email protected]> * Update shrinker-test/src/main/java/com/example/NoSerializedNameMain.java Accept review suggestion Co-authored-by: Marcono1234 <[email protected]> * Further adjust test class/method names for consistency * Update shrinker-test/src/main/java/com/example/NoSerializedNameMain.java Co-authored-by: Marcono1234 <[email protected]> --------- Co-authored-by: Marcono1234 <[email protected]>
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.
R8 can do some optimizations that assume values of types whose constructors are never referenced are
null. (A bit confused why the rules here are sufficient to avoid that optimization in the case of a class whose sole constructor has arguments and is unreferenced, since the keep rule only refers to<init>()instead of<init>(...). Does R8 have special behavior for<init>()where attempting to keep that makes it assume a class is constructable even if it doesn't have a no-args constructor?)Also rename some test classes/names to refer to
NoArgsinstead ofDefaultconstructors. The examples here care about whether a no-argument constructor is present. While a no-argument constructor (explicitly defined or not) is used by default in some circumstances, the Java documentation consistently uses "default constructor" to refer only to constructors whose implementation is implicitly provided by the complier (all default constructors are no-argument constructors, but not vice versa).