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

Functions with default arguments with @boundscheck can be confusing #30411

Open
KristofferC opened this issue Dec 17, 2018 · 2 comments
Open
Assignees
Labels
bug Indicates an unexpected problem or unintended behavior compiler:lowering Syntax lowering (compiler front end, 2nd stage)

Comments

@KristofferC
Copy link
Member

KristofferC commented Dec 17, 2018

function load(a, idx::Int=1)
    @boundscheck checkbounds(a, idx)
    return a[idx]
end

_load(a) = @inbounds load(a)
@code_llvm _load(rand(3))

This will (perhaps confusingly) not elide the boundscheck. Arguably this is an implementation detail of default arguments leaking into the semantics.

@JeffBezanson JeffBezanson self-assigned this Dec 18, 2018
@KristofferC KristofferC added the bug Indicates an unexpected problem or unintended behavior label Aug 2, 2019
@c42f
Copy link
Member

c42f commented Sep 27, 2019

I guess the workaround here is to explicitly write the lowering which the compiler is doing for you, but to add a @propagate_inbounds:

Base.@propagate_inbounds load(a) = load(a,1)

In more generality, I think this is point of confusion for various helper functions which are generated by lowering: they add a layer of indirection which can cause meta information attached by the user to not quite work as expected. (Eg keyword arguments and @inline, possibly the cause of JuliaArrays/StaticArrays.jl#659 (comment)?)

@KristofferC KristofferC changed the title Functions with default arguments with @checkbounds can be confusing Functions with default arguments with @boundscheck can be confusing Feb 23, 2020
@vtjnash vtjnash added the compiler:lowering Syntax lowering (compiler front end, 2nd stage) label Jan 24, 2023
@mbauman
Copy link
Member

mbauman commented Nov 7, 2024

Note @vchuravy's observations in #56493 — in particular that @propagate_inbounds is not itself correct because it goes one level too far, and thus prone to erroneously removing boundschecks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior compiler:lowering Syntax lowering (compiler front end, 2nd stage)
Projects
None yet
Development

No branches or pull requests

5 participants