Skip to content

Commit

Permalink
remove unnecessary true && part from at-view macro (#53064)
Browse files Browse the repository at this point in the history
The modification that expands `@view A[i]` to `true && view(A, i)`
appears to go back as far as #20247. However, I'm not entirely sure why
this is necessary. Considering that just expanding it to `view(A, i)`
still seems to pass the base test suite, I wonder if it might be just
better to get rid of that part.
  • Loading branch information
aviatesk authored Jan 29, 2024
1 parent 5b64a0d commit 0588cd4
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 11 deletions.
23 changes: 12 additions & 11 deletions base/views.jl
Original file line number Diff line number Diff line change
Expand Up @@ -123,20 +123,21 @@ julia> A
```
"""
macro view(ex)
Meta.isexpr(ex, :ref) || throw(ArgumentError(
"Invalid use of @view macro: argument must be a reference expression A[...]."))
ex = replace_ref_begin_end!(ex)
# NOTE We embed `view` as a function object itself directly into the AST.
# By doing this, we prevent the creation of function definitions like
# `view(A, idx) = xxx` in cases such as `@view(A[idx]) = xxx.`
if Meta.isexpr(ex, :ref)
ex = replace_ref_begin_end!(ex)
if Meta.isexpr(ex, :ref)
ex = Expr(:call, view, ex.args...)
else # ex replaced by let ...; foo[...]; end
if !(Meta.isexpr(ex, :let) && Meta.isexpr(ex.args[2], :ref))
error("invalid expression")
end
ex.args[2] = Expr(:call, view, ex.args[2].args...)
end
Expr(:&&, true, esc(ex))
ex = Expr(:call, view, ex.args...)
elseif Meta.isexpr(ex, :let) && (arg2 = ex.args[2]; Meta.isexpr(arg2, :ref))
# ex replaced by let ...; foo[...]; end
ex.args[2] = Expr(:call, view, arg2.args...)
else
throw(ArgumentError("Invalid use of @view macro: argument must be a reference expression A[...]."))
error("invalid expression")
end
return esc(ex)
end

############################################################################
Expand Down
18 changes: 18 additions & 0 deletions test/subarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -937,3 +937,21 @@ end
v = view(1:2, r)
@test v == view(1:2, collect(r))
end

# https://github.com/JuliaLang/julia/pull/53064
# `@view(A[idx]) = xxx` should raise syntax error always
@test try
Core.eval(@__MODULE__, :(@view(A[idx]) = 2))
false
catch err
err isa ErrorException && startswith(err.msg, "syntax:")
end
module Issue53064
import Base: view
end
@test try
Core.eval(Issue53064, :(@view(A[idx]) = 2))
false
catch err
err isa ErrorException && startswith(err.msg, "syntax:")
end

0 comments on commit 0588cd4

Please sign in to comment.