Name collision in @irrational

[hz-xiaxz]

I'm trying to compat https://github.com/jishnub/SphericalHarmonics.jl to IrrationalConstants.jl:0.2.4, however finding this fails.

julia> IrrationalConstants.@irrational _sqrt3by4pi sqrt(3 / (4 * big(pi)))
ERROR: ArgumentError: invalid type for argument  in method definition for #BigFloat#11 at /home/hzxiaxz/.julia/packages/IrrationalConstants/lWTip/src/macro.jl:118
Stacktrace:
 [1] top-level scope
   @ REPL[24]:129
 [2] macro expansion
   @ ~/.julia/packages/IrrationalConstants/lWTip/src/macro.jl:118 [inlined]
 [3] top-level scope
   @ REPL:1

While Base.@irrational still works.

I note there's still implementation difference of the macro here from Base. Should the macro here be updated or I just used the macro wrongly?

[devmotion]

Using Base.@irrational creates a definition of a Irrational{sym} for a new symbol sym, hence using it outside of Base is type piracy.

To avoid this problem, IrrationalConstants uses a different approach: Each irrational is defined as a separate type, and hence it's even fine for multiple packages to define irrationals of the same name as these will be (instances of) separate types.

By default, IrrationalConstants.@irrational uses the name of the irrational as the name of its type, with the first character changed to an uppercase character. However, this default doesn't work in your example as the uppercase version of _sqrt3by4pi is just _sqrt3by4pi. If you inspect the macro output with @macroexpand, you'll see at the top of the generated expression:

julia> @macroexpand IrrationalConstants.@irrational _sqrt3by4pi sqrt(3 / (4 * big(pi)))
quote
    #= /Users/david/.julia/packages/IrrationalConstants/lWTip/src/macro.jl:127 =#
    struct _sqrt3by4pi <: IrrationalConstants.IrrationalConstant
        #= /Users/david/.julia/packages/IrrationalConstants/lWTip/src/macro.jl:127 =#
    end
    #= /Users/david/.julia/packages/IrrationalConstants/lWTip/src/macro.jl:128 =#
    const _sqrt3by4pi = _sqrt3by4pi()

...

This leads to problems in the generated function definitions further down in the expression where we'd like to dispatch on _sqrt3by4pi.

The macro actually allows you to specify the name of the type as an additional argument, so you can work around this problem by using e.g.

julia> IrrationalConstants.@irrational _sqrt3by4pi sqrt(3 / (4 * big(pi))) _Sqrt3by4pi

julia> _sqrt3by4pi
_sqrt3by4pi = 0.4886025119029...

julia> typeof(_sqrt3by4pi)
_Sqrt3by4pi

julia> @macroexpand IrrationalConstants.@irrational _sqrt3by4pi sqrt(3 / (4 * big(pi))) _Sqrt3by4pi

quote
    #= /Users/david/.julia/packages/IrrationalConstants/lWTip/src/macro.jl:127 =#
    struct _Sqrt3by4pi <: IrrationalConstants.IrrationalConstant
        #= /Users/david/.julia/packages/IrrationalConstants/lWTip/src/macro.jl:127 =#
    end
    #= /Users/david/.julia/packages/IrrationalConstants/lWTip/src/macro.jl:128 =#
    const _sqrt3by4pi = _Sqrt3by4pi()
...

I wonder if we should try to improve which type name is chosen by default. On the other hand, if the default is too different from the name of the irrational, it might also lead to surprising problems (eg if a variable of that name already exists).

[hz-xiaxz]

Thank you David! I didn't know @macroexpand so got confused. For the type name, what about something like var_irr, that is appending a _irr suffix. If a Camel convention is preferred, maybe adding Irr etc. That convention may have negletible chance of name collision?

[hz-xiaxz]

This is a successful work around. Thanks for the help!

[devmotion]

Maybe the cleanest solution is to throw a descriptive error if both name and type of the irrational are the same, and tell users to users a different name or to specify the type with an additional argument.