Issue metadata
Sign in to add a comment
|
java assert is not working |
||||||||||||||||||||
Issue descriptionJava asserts seem to have stopped working. A simple "assert false" does not have any effect in my local builds, and not in those of bauerb either. I'm following the guidance in https://chromium.googlesource.com/chromium/src/+/master/styleguide/java/java.md I've tried with these GN args: is_debug = true dcheck_always_on = true (which should not be necessary with is_debug = true) Adding some people from the original feature issue 462676 . Device: Pixel XL OS: 8.1.0 userdebug dev-keys
,
Jun 26 2018
Couldn't find any relevant changes but I noticed that the bytecode rewritten .jars did have asserts enabled but then this code was removed in the dex file. Turns out this is caused by switching to d8 for dexing: https://chromium-review.googlesource.com/c/chromium/src/+/1079373
,
Jun 26 2018
Setting disableAssertions = false in https://r8.googlesource.com/r8/+/master/src/main/java/com/android/tools/r8/utils/InternalOptions.java#211 leaves asserts in the dex file. We can update our d8.jar with the fix and file a bug with d8 to add a --disable-assertions flag.
,
Jun 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/70ed356560528473397935e192d7f293a84c0fee commit 70ed356560528473397935e192d7f293a84c0fee Author: Eric Stevenson <estevenson@chromium.org> Date: Tue Jun 26 21:21:46 2018 Android: Update D8 with fix to enable Java asserts. We switched to D8 in 2e3db78141260081f2da2a849659675666613e98, but it turns out that D8 doesn't support Java asserts. This CL updates D8 with a fix to allow our assertion-enabling code to work again. Bug: 856575 Change-Id: I8f0f239c40de503445bc6a24ad54449a6b5de18c Reviewed-on: https://chromium-review.googlesource.com/1115424 Reviewed-by: agrieve <agrieve@chromium.org> Commit-Queue: Eric Stevenson <estevenson@chromium.org> Cr-Commit-Position: refs/heads/master@{#570534} [modify] https://crrev.com/70ed356560528473397935e192d7f293a84c0fee/DEPS [modify] https://crrev.com/70ed356560528473397935e192d7f293a84c0fee/third_party/r8/README.chromium
,
Jun 27 2018
Thanks for the quick fix! Yes, some kind of test would be nice.
,
Jun 27 2018
Filed b/110887293 with D8. I'll add a test, but I think it'll only work for debug builds unless we add a proguard config for it?
,
Jun 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/665b9b609ea3f608e88a9f9109485247e02fd2a8 commit 665b9b609ea3f608e88a9f9109485247e02fd2a8 Author: Eric Stevenson <estevenson@chromium.org> Date: Wed Jun 27 21:40:12 2018 Android: Add a test to ensure asserts are working. Recently asserts stopped working when we switched to D8 for dexing. This CL adds a test that ensures: * asserts fire when DCHECK is on * asserts are removed when DCHECK is off Bug: 856575 Change-Id: I148e4225fd90709f7738150a73636a3d9f298230 Reviewed-on: https://chromium-review.googlesource.com/1117570 Reviewed-by: agrieve <agrieve@chromium.org> Commit-Queue: Eric Stevenson <estevenson@chromium.org> Cr-Commit-Position: refs/heads/master@{#570907} [modify] https://crrev.com/665b9b609ea3f608e88a9f9109485247e02fd2a8/base/BUILD.gn [add] https://crrev.com/665b9b609ea3f608e88a9f9109485247e02fd2a8/base/android/javatests/src/org/chromium/base/AssertsTest.java
,
Jun 27 2018
|
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by agrieve@chromium.org
, Jun 26 2018Owner: estevenson@chromium.org