Skip to content
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

Base.mightalias claims to be conservative but isn't #50820

Open
jakobnissen opened this issue Aug 7, 2023 · 1 comment
Open

Base.mightalias claims to be conservative but isn't #50820

jakobnissen opened this issue Aug 7, 2023 · 1 comment
Assignees
Labels
arrays [a, r, r, a, y, s] docs This change adds or pertains to documentation

Comments

@jakobnissen
Copy link
Contributor

Base.mightalias 's docstring says (my emphasis)

Perform a conservative test to check if arrays A and B might share the same memory.

Yet, its default implementation basically always returns false , even for objects that does alias:

julia> struct Foo <: AbstractVector{Int}
           x::Vector{Int}
       end;

julia> x = [1,2];

julia> Base.mightalias(x, Foo(x))
false

julia> d = Dict();

julia> Base.mightalias(d, d.keys)
false

I think this is a problem. Presumably, mightalias is going to be used as a guard to prevent aliasing bugs. Having it claim to be conservative, but actually falsely return false for most arguments is a huge footgun.

Luckily for this issue, Base.mightalias does not appear to be documented in the official Julia docs, so it's fair game to change. I believe the best course of action is to remove the default fallback implementation(s) for this and Base.dataids.

See also: #26865

@mbauman
Copy link
Member

mbauman commented Aug 7, 2023

Yes, the aliasing detection system is opt-in. Once you opt-in, it is a conservative check. Perhaps that one-sentence summary should be clearer, but the rest of the docstring is trying to describe this:

By default, this simply checks if either of the arrays reference the same memory regions, as identified by their Base.dataids.

I fought for a long time trying to come up with a better default fallback, but it is extraordinarily hard to do so. See #26237. Removing the default fallbacks is highly breaking, even in situations where no aliasing occurs.

@brenhinkeller brenhinkeller added arrays [a, r, r, a, y, s] docs This change adds or pertains to documentation and removed arrays [a, r, r, a, y, s] labels Aug 7, 2023
@nsajko nsajko added the arrays [a, r, r, a, y, s] label May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrays [a, r, r, a, y, s] docs This change adds or pertains to documentation
Projects
None yet
Development

No branches or pull requests

4 participants