-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Disagreements in token type are not handled correctly #30
Labels
bug
Something isn't working
Comments
Regarding this and #29, in ViewBinding the resource processing was split into three phases:
We probably need a similar pipeline here. It may require two models, at least partially. Otherwise you might lose some info. |
JakeWharton
added a commit
that referenced
this issue
Jan 17, 2023
For strings which match, this reduces the generated bytecode impact. Each argument 'put' operation in the map required: 36: aload 6 38: ldc #99 // String 0 40: aload_1 41: invokevirtual #30 // Method android/util/ArrayMap.put:(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object; 44: pop plus the associated string pool constant's size. In the array, storage requires fewer bytecodes and bytecodes with fewer arguments reducing their size: 32: aload 7 34: iconst_0 35: aload_1 36: aastore Creating the map used to cost 10 bytes and require invoking the ArrayMap constructor: 26: new #23 // class android/util/ArrayMap 29: dup 30: iconst_5 31: invokespecial #26 // Method android/util/ArrayMap."<init>":(I)V 34: astore 6 But the array requires only 5 bytes and zero method calls: 27: anewarray #4 // class java/lang/Object 30: astore 7 Finally, previously, the `Map` property of the `FormattedResource` class required an explicit cast inside the generated code: 89: new #32 // class app/cash/gingham/FormattedResource 92: dup 93: ldc #108 // int 2131689631 95: aload 6 97: checkcast #35 // class java/util/Map 100: invokespecial #38 // Method app/cash/gingham/FormattedResource."<init>":(ILjava/util/Map;)V 103: areturn But with the property changing to `Any` (i.e., `Object`) the cast is no longer required for either the array-based or map-based generated code: 69: new #32 // class app/cash/gingham/FormattedResource 72: dup 73: ldc #96 // int 2131689631 75: aload 6 77: invokespecial #36 // Method app/cash/gingham/FormattedResource."<init>":(ILjava/lang/Object;)V 80: areturn
This is actually a problem within a single configuration, too: <string name="hi">{count,date} {count,select}</string> |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
res/values/strings.xml
:res/values-es/strings.xml
:Currently this produces:
I suspect we simply want to enforce the types to match across all configurations and fail the build otherwise.
Are there any cases where you might want to have multiple types for a single token? Solutions to this depend on #29 a bit, since a workaround for that would be to name them differently (provided we allow this with an XML attribute somehow). Then at the callsite you would have to do
library_text_argument(nameDate = someInstance, nameNumber = 12)
.The text was updated successfully, but these errors were encountered: