Skip to content

@nospecialize for string_index_err #57604

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

nsajko
Copy link
Contributor

@nsajko nsajko commented Mar 2, 2025

The fields of StringIndexError are abstractly typed, so there's no reason to specialize on a concrete type, I think. The change seems like it could prevent some invalidation on loading user code.

@nsajko nsajko added error handling Handling of exceptions by Julia or the user strings "Strings!" invalidations labels Mar 2, 2025
@nsajko nsajko force-pushed the Base_strings_StringIndexError_nospecialize branch from 7e082a5 to ae2ee2a Compare March 2, 2025 11:33
@nsajko nsajko added the DO NOT MERGE Do not merge this PR! label Mar 2, 2025
@LilithHafner LilithHafner marked this pull request as draft March 2, 2025 12:49
@nsajko nsajko removed the DO NOT MERGE Do not merge this PR! label Mar 2, 2025
@nsajko nsajko marked this pull request as ready for review March 2, 2025 13:53
@nsajko
Copy link
Contributor Author

nsajko commented Mar 2, 2025

The change decreases the invalidation count on loading package TypeDomainNaturalNumbers v6.1.0 (without first loading the REPL) from 989 to 980 needs updating.

Reproducer:

./julia -E 'using SnoopCompileCore; i=(@snoop_invalidations using TypeDomainNaturalNumbers); using SnoopCompile; length(uinvalidated(i))'

@nsajko nsajko force-pushed the Base_strings_StringIndexError_nospecialize branch from 8044dba to dc76a47 Compare March 3, 2025 12:17
nsajko added 3 commits March 8, 2025 20:53
The fields of `StringIndexError` are abstractly typed, so there's no
reason to specialize, I think. The change seems like it could prevent
some invalidation on loading user code.
@nsajko nsajko force-pushed the Base_strings_StringIndexError_nospecialize branch from 4a45773 to 7f373e3 Compare March 8, 2025 19:54
@nsajko nsajko changed the title @nospecialize for string_index_err and StringIndexError @nospecialize for string_index_err Mar 8, 2025
@nsajko nsajko added backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 backport 1.12 Change should be backported to release-1.12 labels Mar 9, 2025
This was referenced Mar 11, 2025
This was referenced Mar 20, 2025
@KristofferC KristofferC mentioned this pull request Mar 31, 2025
36 tasks
@@ -9,7 +9,7 @@ struct StringIndexError <: Exception
string::AbstractString
index::Integer
Copy link
Member

Choose a reason for hiding this comment

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

Looking at string_index_err this could be made ::Int I think?

@KristofferC KristofferC mentioned this pull request Apr 4, 2025
51 tasks
@KristofferC KristofferC mentioned this pull request Apr 25, 2025
42 tasks
@KristofferC KristofferC mentioned this pull request Apr 29, 2025
44 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 backport 1.12 Change should be backported to release-1.12 error handling Handling of exceptions by Julia or the user invalidations strings "Strings!"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants