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

Wrong assumption about @doc arguments breaks Revise #538

Closed
mgkurtz opened this issue Jul 14, 2022 · 5 comments
Closed

Wrong assumption about @doc arguments breaks Revise #538

mgkurtz opened this issue Jul 14, 2022 · 5 comments

Comments

@mgkurtz
Copy link
Contributor

mgkurtz commented Jul 14, 2022

When using Revise, I got the following error

┌ Error: Failed to revise /some/path/to/file.jl
│   exception =
│    BoundsError: attempt to access 3-element Vector{Any} at index [4]
│    Stacktrace:
│     [1] getindex
│       @ ./array.jl:861 [inlined]
│     [2] iterate(iter::JuliaInterpreter.ExprSplitter, state::Nothing)
│       @ JuliaInterpreter ~/.julia/packages/JuliaInterpreter/Q7nmG/src/construct.jl:526

which was generated by

body = ex.args[4]

There it is assumed, that if is_doc_expr(ex) holds, ex.args must have length 4. But I have foolishly added a comment between my docstring and the function I documented, like

@doc "foo"
# TODO: implement this correctly ;)
foo() = 42

which resulted in @doc having only one argument and ex.args = [:@doc, LineNumberNode(...), "TODO ..."] thus has only length 3. So maybe adding this comment was foolish from me, but my follys should not break Revise from working. To settle this, perhaps is_doc_expr may get an extra line length(ex.args) == 4 || return false?

@simeonschaub
Copy link
Collaborator

To settle this, perhaps is_doc_expr may get an extra line length(ex.args) == 4 || return false?

This is probably reasonable either way, but that comments in the line after @doc parse this way seems like an upstream bug to me. Could you open an issue for this on the Julia repo?

@pfitzseb
Copy link
Member

There's a typo in the OP; the comment doesn't show up anywhere after parsing:

julia> Meta.parseall("""
       @doc "foo"
       # TODO: implement this correctly ;)
       foo() = 42 
       """) |> dump
Expr
  head: Symbol toplevel
  args: Array{Any}((4,))
    1: LineNumberNode
      line: Int64 1
      file: Symbol none
    2: Expr
      head: Symbol macrocall
      args: Array{Any}((3,))
        1: Symbol @doc
        2: LineNumberNode
          line: Int64 1
          file: Symbol none
        3: String "foo"
    3: LineNumberNode
      line: Int64 3
      file: Symbol none
    4: Expr
      head: Symbol =
      args: Array{Any}((2,))
        1: Expr
          head: Symbol call
          args: Array{Any}((1,))
            1: Symbol foo
        2: Expr
          head: Symbol block
          args: Array{Any}((2,))
            1: LineNumberNode
              line: Int64 3
              file: Symbol none
            2: Int64 42

@simeonschaub
Copy link
Collaborator

Ah, that makes a lot more sense! In that case the check should be all that's needed

@mgkurtz
Copy link
Contributor Author

mgkurtz commented Jul 16, 2022

There's a typo in the OP; the comment doesn't show up anywhere after parsing:

Sorry, perhaps I should have provided a dump as well. Of course the comments do not show up anywhere, but also the to be documented object does not show up in the @doc macrocall:

julia> Meta.parseall("""
       @doc "foo"
       # TODO: implement this correctly ;)
       foo() = 42 
       """) |> dump
[...]
    2: Expr
      head: Symbol macrocall
      args: Array{Any}((3,))
        1: Symbol @doc
        2: LineNumberNode
          line: Int64 1
          file: Symbol none
        3: String "foo"
[...]

which is actually the documented behavior. To be honest, I was actually quite confused by the fact, that the complete dump contained four arguments 😕 But the @doc macrocall, I quoted above, really has only three arguments and the fourth argument of the surrounding expression should better be in there instead of outside.

[...] that comments in the line after @doc parse this way seems like an upstream bug to me. Could you open an issue for this on the Julia repo?

Well, as it is documented, it is technically not a bug, but still strange: this code results in @doc "foo" returning the documentation of String, which is then ignored. If the @doc were not there, it would simply result in a line containing "foo", which would be ignored. I opened an upstream issue to discuss that topic.

mgkurtz added a commit to mgkurtz/JuliaInterpreter.jl that referenced this issue Jul 17, 2022
timholy pushed a commit that referenced this issue Jul 24, 2022
@timholy
Copy link
Member

timholy commented Jul 24, 2022

Closed by #539

@timholy timholy closed this as completed Jul 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants