Including source file information with residual names that overlap input names is not correctly represented [279702361]

Fixed

Bug

Status Update

No update yet.

Description

ze...@google.com

created issue #1

Apr 26, 2023 07:56AM

Implementing b/214018111 shows an issue found in ClassNameMinifierOriginalClassNameTest.

Here the expected encoding of the mapping would become:

com.android.tools.r8.naming.testclasses.A -> com.android.tools.r8.naming.testclasses.A:
# {"id":"sourceFile","fileName":"A.java"}

com.android.tools.r8.naming.testclasses.B -> com.android.tools.r8.naming.testclasses.A:
    1:1:void com.android.tools.r8.naming.testclasses.A.<init>():0:0 -> <init>
    1:1:void <init>():0 -> <init>
    void foo() -> a

But the overlap in residual name A causes the map to be clobbered. The source for original type A is needed, but it has no residual output so the mapping should be something like:

com.android.tools.r8.naming.testclasses.A -> removed:
# {"id":"sourceFile","fileName":"A.java"}

com.android.tools.r8.naming.testclasses.B -> com.android.tools.r8.naming.testclasses.A:
    1:1:void com.android.tools.r8.naming.testclasses.A.<init>():0:0 -> <init>
    1:1:void <init>():0 -> <init>
    void foo() -> a

Comments

ap...@google.com <ap...@google.com> #2Apr 26, 2023 09:08AM

Gradle issue asking for it to be made easier to avoid making an unused modification to this list: https://github.com/gradle/gradle/issues/22671

ap...@google.com <ap...@google.com> #3Apr 26, 2023 09:08AM

I think the issue here is that jvmArgs is null, which causes addAll() to not run.

IMO we should be adding to jvmArgs and not replacing it entirely, in case something else has previously set jvmArgs. WYT @Ivan?

I could change it to something like

forkOptions.jvmArgs?.addAll(executionOptions.get().jvmArgs) ?: forkOptions.jvmArgs(executionOptions.get().jvmArgs)

ap...@google.com <ap...@google.com> #4Apr 27, 2023 07:37AM

Yeah, the nullness of jvmArgs was the first possibility I checked too, although even with "!!" it still had no effect, see https://github.com/gradle/gradle/issues/22671

I uploaded a change that I observed to fix it for me and added you as a reviewer

ap...@google.com <ap...@google.com> #5May 1, 2023 12:08PM

Ah, ok, that makes sense.

ap...@google.com <ap...@google.com> #6May 1, 2023 12:08PM

Thanks for the change, Jeff!

Should we cherry-pick it into 7.4? I think AGP 7.4 has that but the settings plugin is not officially supported yet.

ap...@google.com <ap...@google.com> #7May 1, 2023 12:08PM

Cherry-pick to AGP 7.4 branch is here. It's not merged yet, and I don't have an opinion about whether it should be.

Can we add a regression test in studio-main for this?

ap...@google.com <ap...@google.com> #8May 1, 2023 12:08PM

Chờ nhận sp

ap...@google.com <ap...@google.com> #9May 1, 2023 12:10PM

Project: r8
Branch: 8.1

commit 5d818f36738012e7a46db7676ec8cfb9ca5d815c
Author: Morten Krogh-Jespersen <mkroghj@google.com>
Date: Mon May 01 11:35:07 2023

Fix clobbering in mapping when recording sourcefile for pruned classes

Bug:

b/279702361

Change-Id: Ia2895fa2597ba5264ee420c2a09a5eacc37f7509

M src/main/java/com/android/tools/r8/naming/ClassNameMapper.java
M src/main/java/com/android/tools/r8/naming/SeedMapper.java
M src/main/java/com/android/tools/r8/utils/positions/MappedPositionToClassNameMapperBuilder.java
M src/test/java/com/android/tools/r8/naming/PackageNamingTest.java
M src/test/java/com/android/tools/r8/utils/codeinspector/CodeInspector.java

https://r8-review.googlesource.com/78920

ap...@google.com <ap...@google.com> #10May 2, 2023 08:41AM

Project: r8
Branch: 8.0

commit d2a07998920bb56d3e6f401affc6e6e79120879b
Author: Morten Krogh-Jespersen <mkroghj@google.com>
Date: Tue May 02 09:01:36 2023

Version 8.0.43

This applies the CLs from

b/270137541

to resolve status changes introduced by cherry-picking

b/279702361

.

Bug:

b/279702361

Change-Id: I91b4946fda02f1ca2b3c2287bf4dc07bfe121ae1

M src/main/java/com/android/tools/r8/Version.java

https://r8-review.googlesource.com/78944

sg...@google.com <sg...@google.com> #11May 8, 2023 04:22PM

Marked as fixed.

Landed with 8.1.43 in Studio Giraffe.

gr...@gmail.com <gr...@gmail.com> #12Mar 9, 2024 09:40AM

Mendarat dengan 8.1.43 di Studio Giraffe.

gr...@gmail.com <gr...@gmail.com> #13Mar 9, 2024 09:41AM

Proyek: r8 Cabang: 8.0 commit e16a8b582254fb3db39d194d6826316ddd35f737 Penulis: Morten Krogh-Jespersen mkroghj@google.com Tanggal: Sen 01 Mei 12:35:28 2023 Memperbaiki gangguan dalam pemetaan saat merekam file sumber untuk kelas yang dipangkas Bug: b/279702361 Ubah-Id : Ia2895fa2597ba5264ee420c2a09a5eacc37f7509 M src/main/java/com/android/tools/r8/naming/ClassNameMapper.java M src/main/java/com/android/tools/r8/naming/SeedMapper.java M src/main/java/com /android/tools/r8/utils/positions/MappedPositionToClassNameMapperBuilder.java M src/test/java/com/android/tools/r8/naming/PackageNamingTest.java M src/test/java/com/android/tools/r8/utils /codeinspector/CodeInspector.java https://r8-review.googlesource.com/78881

Issue 279702361

Description

Issue summary

Comments

ap...@google.com <ap...@google.com> #2Apr 26, 2023 09:08AM

ap...@google.com <ap...@google.com> #3Apr 26, 2023 09:08AM

ap...@google.com <ap...@google.com> #4Apr 27, 2023 07:37AM

ap...@google.com <ap...@google.com> #5May 1, 2023 12:08PM

ap...@google.com <ap...@google.com> #6May 1, 2023 12:08PM

ap...@google.com <ap...@google.com> #7May 1, 2023 12:08PM

ap...@google.com <ap...@google.com> #8May 1, 2023 12:08PM

ap...@google.com <ap...@google.com> #9May 1, 2023 12:10PM

ap...@google.com <ap...@google.com> #10May 2, 2023 08:41AM

sg...@google.com <sg...@google.com> #11May 8, 2023 04:22PM

gr...@gmail.com <gr...@gmail.com> #12Mar 9, 2024 09:40AM

gr...@gmail.com <gr...@gmail.com> #13Mar 9, 2024 09:41AM

Add comment

Issue metadata