Skip to content

Conversation

nsajko
Copy link
Member

@nsajko nsajko commented Mar 22, 2025

Should make the sysimage less vulnerable to invalidation.

As far as I understand this change can't break any code, because:

  • the macro definition requires AbstractString
  • the way Julia macros work, the argument is either a literal or an Expr
  • String is the only AbstractString with literals

Should make the sysimage less vulnerable to invalidation.

As far as I understand this change can't break any code, because:
* the macro definition requires `AbstractString`
* the way Julia macros work, the argument is either a literal or an `Expr`
* `String` is the only `AbstractString` with literals
@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 invalidations backport 1.12 Change should be backported to release-1.12 labels Mar 22, 2025
@nsajko nsajko changed the title Base: macro cmd restrict argument to String Base: macro cmd: restrict argument to String Mar 22, 2025
@nsajko nsajko added the macros @macros label Mar 22, 2025
@nsajko
Copy link
Member Author

nsajko commented Mar 22, 2025

Reduces the invalidation count on loading TypeDomainNaturalNumbers v7.0.0 from 739 to 731, preventing these invalidations:

{
    "method_instance": {
        "method": "iterate(x::Base.Iterators.IterableStatePairs, state) @ Base.Iterators iterators.jl:1586",
        "method_instance": "MethodInstance for iterate(::Base.Iterators.IterableStatePairs{T} where T<:SubString, ::Any)"
    },
    "children": [
        {
            "method_instance": {
                "method": "popfirst!(s::Base.Iterators.Stateful) @ Base.Iterators iterators.jl:1487",
                "method_instance": "MethodInstance for popfirst!(::Base.Iterators.Stateful{T} where T<:(Base.Iterators.IterableStatePairs{T} where T<:SubString))"
            },
            "children": [
                {
                    "method_instance": {
                        "method": "iterate(s::Base.Iterators.Stateful, state) @ Base.Iterators iterators.jl:1502",
                        "method_instance": "MethodInstance for iterate(::Base.Iterators.Stateful{T} where T<:(Base.Iterators.IterableStatePairs{T} where T<:SubString), ::Nothing)"
                    },
                    "children": [
                        {
                            "method_instance": {
                                "method": "iterate(s::Base.Iterators.Stateful) @ Base.Iterators iterators.jl:1502",
                                "method_instance": "MethodInstance for iterate(::Base.Iterators.Stateful{T} where T<:(Base.Iterators.IterableStatePairs{T} where T<:SubString))"
                            },
                            "children": [
                                {
                                    "method_instance": {
                                        "method": "var\"#shell_parse#475\"(special::AbstractString, filename, ::typeof(Base.shell_parse), str::AbstractString, interpolate::Bool) @ Base shell.jl:31",
                                        "method_instance": "MethodInstance for Base.var\"#shell_parse#475\"(::String, ::String, ::typeof(Base.shell_parse), ::AbstractString, ::Bool)"
                                    },
                                    "children": [
                                        {
                                            "method_instance": {
                                                "method": "kwcall(::NamedTuple, ::typeof(Base.shell_parse), str::AbstractString, interpolate::Bool) @ Base shell.jl:31",
                                                "method_instance": "MethodInstance for Core.kwcall(::@NamedTuple{special::String, filename::String}, ::typeof(Base.shell_parse), ::AbstractString, ::Bool)"
                                            },
                                            "children": [
                                                {
                                                    "method_instance": {
                                                        "method": "kwcall(::NamedTuple, ::typeof(Base.shell_parse), str::AbstractString) @ Base shell.jl:31",
                                                        "method_instance": "MethodInstance for Core.kwcall(::@NamedTuple{special::String, filename::String}, ::typeof(Base.shell_parse), ::AbstractString)"
                                                    },
                                                    "children": [
                                                        {
                                                            "method_instance": {
                                                                "method": "var\"@cmd\"(__source__::LineNumberNode, __module__::Module, str) @ Base cmd.jl:507",
                                                                "method_instance": "MethodInstance for Core.var\"@cmd\"(::LineNumberNode, ::Module, ::Any)"
                                                            },
                                                            "children": [
                                                            ]
                                                        }
                                                    ]
                                                }
                                            ]
                                        }
                                    ]
                                }
                            ]
                        },
                        {
                            "method_instance": {
                                "method": "var\"#shell_parse#475\"(special::AbstractString, filename, ::typeof(Base.shell_parse), str::AbstractString, interpolate::Bool) @ Base shell.jl:31",
                                "method_instance": "MethodInstance for Base.var\"#shell_parse#475\"(::String, ::String, ::typeof(Base.shell_parse), ::AbstractString, ::Bool)"
                            },
                            "children": [
                            ]
                        }
                    ]
                },
                {
                    "method_instance": {
                        "method": "var\"#shell_parse#475\"(special::AbstractString, filename, ::typeof(Base.shell_parse), str::AbstractString, interpolate::Bool) @ Base shell.jl:31",
                        "method_instance": "MethodInstance for Base.var\"#shell_parse#475\"(::String, ::String, ::typeof(Base.shell_parse), ::AbstractString, ::Bool)"
                    },
                    "children": [
                    ]
                }
            ]
        }
    ]
}

@nsajko
Copy link
Member Author

nsajko commented Mar 23, 2025

A more minimal reproducer than using TypeDomainNaturalNumbers is:

struct Z <: Integer end
Base.:(+)(n::Int, ::Z) = n

@KristofferC KristofferC mentioned this pull request Mar 24, 2025
27 tasks
@KristofferC KristofferC merged commit 7acb2a6 into JuliaLang:master Mar 25, 2025
7 of 9 checks passed
@nsajko nsajko deleted the Base_restrict_macro_cmd_argument_to_String branch March 25, 2025 13:05
KristofferC pushed a commit that referenced this pull request Mar 26, 2025
Should make the sysimage less vulnerable to invalidation.

As far as I understand this change can't break any code, because:
* the macro definition requires `AbstractString`
* the way Julia macros work, the argument is either a literal or an
`Expr`
* `String` is the only `AbstractString` with literals

(cherry picked from commit 7acb2a6)
@KristofferC KristofferC removed the backport 1.12 Change should be backported to release-1.12 label Mar 31, 2025
KristofferC pushed a commit that referenced this pull request Mar 31, 2025
Should make the sysimage less vulnerable to invalidation.

As far as I understand this change can't break any code, because:
* the macro definition requires `AbstractString`
* the way Julia macros work, the argument is either a literal or an
`Expr`
* `String` is the only `AbstractString` with literals

(cherry picked from commit 7acb2a6)
@KristofferC KristofferC mentioned this pull request Mar 31, 2025
71 tasks
KristofferC pushed a commit that referenced this pull request Mar 31, 2025
Should make the sysimage less vulnerable to invalidation.

As far as I understand this change can't break any code, because:
* the macro definition requires `AbstractString`
* the way Julia macros work, the argument is either a literal or an
`Expr`
* `String` is the only `AbstractString` with literals

(cherry picked from commit 7acb2a6)
KristofferC pushed a commit that referenced this pull request Mar 31, 2025
Should make the sysimage less vulnerable to invalidation.

As far as I understand this change can't break any code, because:
* the macro definition requires `AbstractString`
* the way Julia macros work, the argument is either a literal or an
`Expr`
* `String` is the only `AbstractString` with literals

(cherry picked from commit 7acb2a6)
@KristofferC KristofferC removed the backport 1.11 Change should be backported to release-1.11 label Apr 10, 2025
@KristofferC KristofferC mentioned this pull request Jun 4, 2025
75 tasks
KristofferC pushed a commit that referenced this pull request Jun 5, 2025
Should make the sysimage less vulnerable to invalidation.

As far as I understand this change can't break any code, because:
* the macro definition requires `AbstractString`
* the way Julia macros work, the argument is either a literal or an
`Expr`
* `String` is the only `AbstractString` with literals

(cherry picked from commit 7acb2a6)
KristofferC pushed a commit that referenced this pull request Jul 3, 2025
Should make the sysimage less vulnerable to invalidation.

As far as I understand this change can't break any code, because:
* the macro definition requires `AbstractString`
* the way Julia macros work, the argument is either a literal or an
`Expr`
* `String` is the only `AbstractString` with literals

(cherry picked from commit 7acb2a6)
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 invalidations macros @macros

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants