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

Revising "docstring" f(::A{T}) where T fails with semi-helpful error message #735

Closed
mgkurtz opened this issue Feb 23, 2023 · 2 comments · Fixed by #774
Closed

Revising "docstring" f(::A{T}) where T fails with semi-helpful error message #735

mgkurtz opened this issue Feb 23, 2023 · 2 comments · Fixed by #774

Comments

@mgkurtz
Copy link

mgkurtz commented Feb 23, 2023

I can write code like

module Foo
struct A{T} end
"""Magic function"""
f(::A{T}) where T
end

This is valid Julia code, but Revise cannot revise it and gives a not-so-helpful error message.

Steps to reproduce: write the above code in /path/to/file/Foo.jl. Start the Julia REPL and type

`push!(LOAD_PATH, "/path/to/file")`
using Foo

Then add a line like x=42 to Foo.jl and try to access it in the REPL. You will get

┌ Error: Failed to revise /path/to/file/Foo.jl
│   exception = lowering returned an error, :($(Expr(:error, "invalid \"::\" syntax")))
└ @ Revise ~/.julia/packages/Revise/P8ITN/src/packagedef.jl:717

This is semi-helpfull. Working on a large file written by others, I had some trouble pin-pointing this error. Changing methods_by_execution! to also print the statement producing the error, I got

exception = lowering returned an error, (:($(Expr(:error, "invalid \"::\" syntax"))), :(f(::A{T}) where T))

Now, we see, the error arises when methods_by_execution! calls Meta.lower(Foo, :(f(::A{T} where T))). Without context f(::A{T}) where T evaluates to error("invalid \"::\" syntax"). Hence appropriately Meta.lower gives $(Expr(:error, "invalid \"::\" syntax")).

For some reason, the error occurs, when the argument to f is parametric but not, when I just document f(::Int). Although f(::Int) evaluates to the same “invalid "::" syntax” error as f(::A{T}) where T does. Curious.

Since working around that bug is rather easy (add an x in front of ::A{T}), I would be happy enough, if I we could provide some useful error message.

mgkurtz added a commit to mgkurtz/AbstractAlgebra.jl that referenced this issue Feb 23, 2023
fingolfin pushed a commit to Nemocas/AbstractAlgebra.jl that referenced this issue Feb 23, 2023
fredrikekre added a commit that referenced this issue Nov 9, 2023
This patch adds a (minimal?) targeted hack to workaround an issue where
a method signature for documentation fails lowering (#735).

Specifically, when revising the following code:

```julia
struct Foo{T} end

"docs"
Foo{T}(::Int) where T
```

lowering errors for the expression `:(Foo{T}(::Int) where T)` with
`"invalid :: syntax"`. This patch simply ignores the expression when
this situation is detected. Note that, just after, the full expression
(docstring + signature) is lowered together succesfully. In particular
this means that the docstring will still be updated correctly.
@fredrikekre
Copy link
Collaborator

Just ran into this, thanks for the workaround.

I made a PR to hack around this issue, see #773.

fredrikekre added a commit that referenced this issue Nov 9, 2023
Revise extracts the documented expression and puts that before the full
doc expression in the revision queue. E.g. in `"docs" f(x) = x` there
will be a revision of `f(x) = x` followed by a revision of the full
expresion (`"docs" f(x) = x`). However, in many cases a docstring is
only attached to a signature (not a function body/struct definition,
etc.) and in this case the revision of the signature is not needed.

Revise already filters out the base case, `:("docs" f(x))`. This patch
extends this filtering to also apply to `where`-clauses such as e.g.
`"docs" f(x::T) where T <: Integer`.

Concretely, this patch fixes #735, i.e. revising doc expressions where
the signature doesn't lower without the context of the doc macro. As a
bonus, slightly less work has to be done for e.g. `"docs" f(x::T) where
T` since this is also filtered out.
fredrikekre added a commit that referenced this issue Nov 9, 2023
Revise extracts the documented expression and puts that before the full
doc expression in the revision queue. E.g. in `"docs" f(x) = x` there
will be a revision of `f(x) = x` followed by a revision of the full
expresion (`"docs" f(x) = x`). However, in many cases a docstring is
only attached to a signature (not a function body/struct definition,
etc.) and in this case the revision of the signature is not needed.

Revise already filters out the base case, `"docs" f(x)`. This patch
extends this filtering to also apply to `where`-clauses such as e.g.
`"docs" f(x::T) where T <: Integer`.

Concretely, this patch fixes #735, i.e. revising doc expressions where
the signature doesn't lower without the context of the doc macro. As a
bonus, slightly less work has to be done for e.g. `"docs" f(x::T) where
T` since this is also filtered out.
fredrikekre added a commit that referenced this issue Nov 9, 2023
Revise extracts the documented expression and puts that before the full
doc expression in the revision queue. E.g. in `"docs" f(x) = x` there
will be a revision of `f(x) = x` followed by a revision of the full
expresion (`"docs" f(x) = x`). However, in many cases a docstring is
only attached to a signature (not a function body/struct definition,
etc.) and in this case the revision of the signature is not needed.

Revise already filters out the base case, `"docs" f(x)`. This patch
extends this filtering to also apply to `where`-clauses such as e.g.
`"docs" f(x::T) where T <: Integer`.

Concretely, this patch fixes #735, i.e. revising doc expressions where
the signature doesn't lower without the context of the doc macro. As a
bonus, slightly less work has to be done for e.g. `"docs" f(x::T) where
T` since this is also filtered out.
KristofferC pushed a commit that referenced this issue Nov 10, 2023
* Filter out more doc signature expressions

Revise extracts the documented expression and puts that before the full
doc expression in the revision queue. E.g. in `"docs" f(x) = x` there
will be a revision of `f(x) = x` followed by a revision of the full
expresion (`"docs" f(x) = x`). However, in many cases a docstring is
only attached to a signature (not a function body/struct definition,
etc.) and in this case the revision of the signature is not needed.

Revise already filters out the base case, `"docs" f(x)`. This patch
extends this filtering to also apply to `where`-clauses such as e.g.
`"docs" f(x::T) where T <: Integer`.

Concretely, this patch fixes #735, i.e. revising doc expressions where
the signature doesn't lower without the context of the doc macro. As a
bonus, slightly less work has to be done for e.g. `"docs" f(x::T) where
T` since this is also filtered out.

* Set version to 3.5.8.
@timholy
Copy link
Owner

timholy commented Dec 27, 2023

Nice detective work @mgkurtz and thanks for #774 @fredrikekre!

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

Successfully merging a pull request may close this issue.

3 participants