Skip to content

Conversation

@Alovchin91
Copy link
Member

@Alovchin91 Alovchin91 commented Feb 11, 2025

This PR is split-off from and is a prerequisite of Clang-cl PR: #1020

BreakIterator.clone() is broken on JVM and seemingly leads to a use-after-free behaviour. Clang-cl makes this issue more visible, likely due to address sanitisation.

Explanation why it is deleted in an internal channel: https://jetbrains.slack.com/archives/C02DDNREC77/p1738678000232729

  • breakIteratorCloneTest is failing with an access violation in Debug mode with additional checks, so using it wasn't safe
  • it is not used anywhere

This is a breaking change and will require bumping Skiko version to 0.9.0.

It is broken on JVM and seemingly leads to a use-after-free behaviour
Copy link
Member

@MatkovIvan MatkovIvan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change and will require bumping Skiko version to 0.9.0.

We're still in 0.x versions and usually do not bump the second version because of removing some minor APIs. So I don't mind to bump (it doesn't really matter), but it's not about "will require"

@igordmn igordmn self-requested a review February 11, 2025 14:42
@igordmn
Copy link
Collaborator

igordmn commented Feb 11, 2025

So I don't mind to bump (it doesn't really matter), but it's not about "will require"

I am more for bumping because of changing the compiler on one of the platforms

@MatkovIvan MatkovIvan merged commit 3f95f4e into JetBrains:master Feb 11, 2025
6 checks passed
@Alovchin91 Alovchin91 deleted the alovchin/break-iterator-no-clone branch February 11, 2025 16:10
igordmn pushed a commit that referenced this pull request Feb 12, 2025
Official Skia documentation suggests that it's [_highly
recommended_](https://skia.org/docs/user/build/#highly-recommended-build-with-clang-cl)
to build Skia with Clang-CL on Windows, and that this dramatically
improves Skia performance with Software rendering and in other areas.
This corresponds with my experience, so here it is.

Related Skia-pack PR: JetBrains/skia-pack#64

Prerequisite: #1024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants