From f6a5f87128bcd8932e610fa6484b7889d1607d5c Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki Date: Fri, 26 Jan 2024 16:24:13 +0900 Subject: [PATCH 1/2] remove unnecessary `true &&` part from `at-view` macro 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. --- base/views.jl | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/base/views.jl b/base/views.jl index d83005c86f46c..0792c10088302 100644 --- a/base/views.jl +++ b/base/views.jl @@ -123,20 +123,18 @@ 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) 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 ############################################################################ From 63ee5209162f0567beb7ca548fa2785a2d02bba0 Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki Date: Sat, 27 Jan 2024 13:13:51 +0900 Subject: [PATCH 2/2] add comment and tests --- base/views.jl | 3 +++ test/subarray.jl | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/base/views.jl b/base/views.jl index 0792c10088302..6898abdda1471 100644 --- a/base/views.jl +++ b/base/views.jl @@ -126,6 +126,9 @@ 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 = Expr(:call, view, ex.args...) elseif Meta.isexpr(ex, :let) && (arg2 = ex.args[2]; Meta.isexpr(arg2, :ref)) diff --git a/test/subarray.jl b/test/subarray.jl index ad68496e38245..40877d842df76 100644 --- a/test/subarray.jl +++ b/test/subarray.jl @@ -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