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

RFC: Documenting macro-generated code #13006

Merged
merged 1 commit into from
Sep 29, 2015

Conversation

MichaelHatherly
Copy link
Member

This adds a @__doc__ macro, not exported currently, for use by macro authors that want their macros to hook into the docsystem properly.

Usage:

macro example(f)
    quote
        $(f)() = 0
        @__doc__ $(f)(x) = 1
        $(f)(x, y) = 2
    end |> esc
end

"Docs for `g(x)` only."
@example g

will attach the docstring to the 1-arg method only.

Fixes #12705.

@kshyatt kshyatt added the docsystem The documentation building system label Sep 8, 2015
@MichaelHatherly MichaelHatherly force-pushed the mh/at__doc__ branch 2 times, most recently from 3621863 to 01a4e32 Compare September 9, 2015 09:30
@MichaelHatherly
Copy link
Member Author

@mauro3, if you happen to have some time to try this out that would be great. The following diff is all that's needed in Traits.jl to allow for documenting @traitdefs:

diff --git a/src/traitdef.jl b/src/traitdef.jl
index 70d447d..efdc23c 100644
--- a/src/traitdef.jl
+++ b/src/traitdef.jl
@@ -277,7 +277,7 @@ macro traitdef(head, body)
         methods::Traits.FDict
         constraints::Vector{Bool}
         assoctyps::Vector{Any}
-        function $((name))()
+        Docs.@__doc__ function $((name))()
             $assoc
             new( $meths, $constr, assoctyps)
         end

and then docs for traits should work automatically

julia> """
           MyArith{X, Y}

       ...
       """
       @traitdef MyArith{X,Y} begin
           # associated types
           Z = promote_type(X,Y)
           D = (X,Y)<:(Integer, Integer) ? Float64 : Z

           # method signatures
           +(X,Y) -> Z
           -(X,Y) -> Z
           *(X,Y) -> Z
           /(X,Y) -> D

           # constraints on X,Y
           @constraints begin
               X<:Number
               Y<:Number
           end
       end

help?> MyArith
search: MyArith

  MyArith{X, Y}

  ...

@MichaelHatherly
Copy link
Member Author

Oops, scratch that last comment, since @traitdef works fine without this... I was meaning to check @with_kw from your Parameters.jl package. It works fine with that:

diff --git a/src/Parameters.jl b/src/Parameters.jl
index 312c42b..821e232 100644
--- a/src/Parameters.jl
+++ b/src/Parameters.jl
@@ -286,7 +286,7 @@ function with_kw(typedef)
     pack_name = symbol("pack_"*string(tn))
     # Finish up
     quote
-        $typ
+        Docs.@__doc__ $typ
         $outer_positional
         $outer_copy
         macro $unpack_name(ex)
julia> "d"
       @with_kw immutable MT1
           r::Int = 4
           c = "sdaf"
       end

help?> MT1
search: MT1 @pack_MT1 @unpack_MT1

  d

Sorry for the noise.

@mauro3
Copy link
Contributor

mauro3 commented Sep 9, 2015

Cool! @traitfn also fails currently, if you need another datapoint:

"asdf" @traitfn tf1{X, Y<:Int; M1Tr100{X}}(a::X, b::Y) = foofoo(a)^b

The __doc__ would need to be attached to
https://github.com/mauro3/Traits.jl/blob/master/src/traitfns.jl#L412

@mauro3
Copy link
Contributor

mauro3 commented Sep 9, 2015

It is cool to get fine-grained control with the __doc__ macro. But would this also work with the "document the return value of the block" by default?

@MichaelHatherly
Copy link
Member Author

if you need another datapoint

Thanks, that one works fine too.

But would this also work with the "document the return value of the block" by default?

I agree with @one-more-minute's comment, #11932 (comment), on this.

Documenting the last thing from generated code block could lead to unintended things getting into __META__ if one does not write a macro to take into account that convention. Requiring this kind of thing to be explicitly opt-in, using @__doc__, seems like the sanest approach to take.

@mauro3
Copy link
Contributor

mauro3 commented Sep 9, 2015

Ok, that sounds good. (And if there is just one thing produced by a macro there is no ambiguity on what to document.) One last thing, is there a better error message now, which suggests to use __doc__?

@MichaelHatherly
Copy link
Member Author

is there a better error message now

Not at the moment. It's still the Invalid doc expression. one. I'll try to add something a little more descriptive shortly.

@MichaelHatherly
Copy link
Member Author

is there a better error message now

Done. Also changed it to display the original expression instead of it's expansion, which should be easier to read.

@mauro3
Copy link
Contributor

mauro3 commented Sep 12, 2015

Will this get back-ported to 0.4?

@StefanKarpinski
Copy link
Member

Seems like a good candidate since there's no way this could break anyone's code.

This adds a `@__doc__` macro, not exported currently, for use by macro authors
that want their macros to hook into the docsystem properly.

Usage:

    macro example(f)
        quote
            $(f)() = 0
            @__doc__ $(f)(x) = 1
            $(f)(x, y) = 2
        end |> esc
    end

    "Docs for `g(x)` only."
    @example g

will attach the docstring to the 1-arg method only.

Also improve invalid doc expression error message by displaying the
unexpanded form of the expression instead of its expansion.

Enable docstrings for `at-enum` using the newly added macro.

Fixes JuliaLang#12705.
@MichaelHatherly
Copy link
Member Author

Yes, it should be fine. I've rebased and squashed the commits. Also enabled documenting @enums directly using this, since that's probably one of the more common macro-generated things for people to what to document.

@mauro3
Copy link
Contributor

mauro3 commented Sep 16, 2015

Although not used a lot to decorate functions directly, maybe @inbounds could be doc-enabled too? (there is also the vectorize macros but probably those aren't necessary. The rest seems to work)

@MichaelHatherly
Copy link
Member Author

maybe @inbounds could be doc-enabled too?

Not sure about needing to support @inbounds. The macro call is put inside the body of a function since it doesn't have an effect when put outside, see code_llvm output of

julia> begin

       @inbounds f(xs, i) = xs[i]

       g(xs, i) = @inbounds return xs[i]

       end
g (generic function with 1 method)

julia> code_llvm(f, Tuple{Vector{Int}, Int})
...
julia> code_llvm(g, Tuple{Vector{Int}, Int})
...

@mauro3
Copy link
Contributor

mauro3 commented Sep 20, 2015

But that is a bug in @inbounds or its documentation:
http://docs.julialang.org/en/release-0.4/manual/performance-tips/?highlight=inbounds#performance-annotations. Opened a bug in #13234.

Also, irrespective, throwing this error may still be to wrong thing to do:

julia> "help" @inbounds f(x)=x
ERROR: invalid doc expression begin 
    $(Expr(:boundscheck, false))
    begin 
        f(x) = begin  # none, line 1:
                x
            end
        $(Expr(:boundscheck, :(Base.pop)))
    end
end

@MichaelHatherly
Copy link
Member Author

Also, irrespective, throwing this error may still be to wrong thing to do:

Yeah, for @inbounds used like that we could mark the first expression in the block with @__doc__ I believe – if it is a bug rather than documentation issue in #13234.

@mauro3
Copy link
Contributor

mauro3 commented Sep 24, 2015

How about we forget about @inbounds for now and merge this? Then, maybe it can still go into 0.4. Or does it need more reviewing/bikesheeding?

@MichaelHatherly
Copy link
Member Author

Could perhaps use a quick look by someone familiar with the docsystem. It is quite a simple addition so getting it into 0.4 would be nice. As Stefan mentioned already, this isn't going to break anything.

I'm not terribly attached to the name @__doc__ so if people want to bikeshed that that's fine with me.

@mauro3
Copy link
Contributor

mauro3 commented Sep 24, 2015

I'll take you up on it: @document or @attachdoc

@tkelman
Copy link
Contributor

tkelman commented Sep 29, 2015

Is this ready or does the name need to change?

@MichaelHatherly
Copy link
Member Author

I'm fine with the name as is so this is ready I'd think.

@mauro3
Copy link
Contributor

mauro3 commented Sep 29, 2015

Yes, it's ready. Sorry for the noise.

tkelman added a commit that referenced this pull request Sep 29, 2015
RFC: Documenting macro-generated code
@tkelman tkelman merged commit f71e449 into JuliaLang:master Sep 29, 2015
@MichaelHatherly
Copy link
Member Author

Thanks for reviewing @mauro3, and @tkelman for all the work backporting things!

@MichaelHatherly MichaelHatherly deleted the mh/at__doc__ branch September 29, 2015 12:12
tkelman pushed a commit that referenced this pull request Sep 30, 2015
This adds a `@__doc__` macro, not exported currently, for use by macro authors
that want their macros to hook into the docsystem properly.

Usage:

    macro example(f)
        quote
            $(f)() = 0
            @__doc__ $(f)(x) = 1
            $(f)(x, y) = 2
        end |> esc
    end

    "Docs for `g(x)` only."
    @example g

will attach the docstring to the 1-arg method only.

Also improve invalid doc expression error message by displaying the
unexpanded form of the expression instead of its expansion.

Enable docstrings for `at-enum` using the newly added macro.

Fixes #12705.

(cherry picked from commit 0b9ef7d)
ref #13006
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docsystem The documentation building system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants