Skip to content

at-evalpoly is broken #23239

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

Closed
stevengj opened this issue Aug 13, 2017 · 8 comments · Fixed by #23247
Closed

at-evalpoly is broken #23239

stevengj opened this issue Aug 13, 2017 · 8 comments · Fixed by #23247
Labels
compiler:lowering Syntax lowering (compiler front end, 2nd stage) macros @macros

Comments

@stevengj
Copy link
Member

stevengj commented Aug 13, 2017

The following works fine in 0.6:

julia> f(x) = @evalpoly x 1.0 2.0 3.0
f (generic function with 1 method)

julia> Base.Test.@inferred f(3.0)
ERROR: return type Float64 does not match inferred return type Any
Stacktrace:
 [1] error(::String) at ./error.jl:28

Probably the same as JuliaMath/SpecialFunctions.jl#42, where it was bisected to #22985.

cc @vtjnash

@stevengj stevengj added compiler:inference Type inference macros @macros labels Aug 13, 2017
@yuyichao yuyichao changed the title broken inference for at-evalpoly at-evalpoly is broken Aug 13, 2017
@yuyichao yuyichao added compiler:lowering Syntax lowering (compiler front end, 2nd stage) and removed compiler:inference Type inference labels Aug 13, 2017
@yuyichao
Copy link
Contributor

This is basically #23221. Right now it's impossible to write macros that calls other macros correctly without doing everything manually.

@stevengj
Copy link
Member Author

As a workaround, we could inline the @horner macro; we could even eliminate that undocumented macro entirely in favor of @evalpoly.

However, in the long run I think we need to make it possible for macros to call macros.

@JeffBezanson
Copy link
Member

From the code_lowered for f:

        Base.Math.t = tt
        SSAValue(0) = (Base.Math.muladd)(Base.Math.t, (Base.Math.muladd)(Base.Math.t, 3.0, 2.0), 1.0)
        # meta: pop location
        return SSAValue(0)

The horner macro returns a block with an assignment to a (hygienic) variable t, which somehow becomes a global. This seems different from the other recent macro issues, since t should be local no matter which scope it belongs to.

@JeffBezanson
Copy link
Member

Reduced example:

julia> macro id(x)
        esc(x)
       end

julia> macro aa()
        quote
         a = 1
         @id b = 1
        end
       end

julia> @aa;

julia> a
ERROR: UndefVarError: a not defined

julia> b
1

I can see why this happens, but I find it really confusing.

vtjnash added a commit that referenced this issue Aug 14, 2017
previously, we were treating hygienic-scope as also introducing a new scope-block
that was wrong (and inconsistent with previous behavior)

also, we were failing to set the outermost flag when entering a
hygienic-scope block, further compounding the above error

fix #23239
@StefanKarpinski
Copy link
Member

vtjnash added a commit that referenced this issue Aug 14, 2017
previously, we were treating hygienic-scope as also introducing a new scope-block
that was wrong (and inconsistent with previous behavior)

also, we were failing to set the outermost flag when entering a
hygienic-scope block, further compounding the above error

fix #23239
vtjnash added a commit that referenced this issue Aug 14, 2017
previously, we were treating hygienic-scope as also introducing a new scope-block
that was wrong (and inconsistent with previous behavior)

also, we were failing to set the outermost flag when entering a
hygienic-scope block, further compounding the above error

fix #23239
@dlfivefifty
Copy link
Contributor

Just came across what's presumably the same bug. On 0.6:

julia> W2 = @evalpoly(0.1729150690306449, @evalpoly(1.2151064605177766, -27.0, -84.0, -56.0),
                                         -11.614665966268532,
                                         163.17740776843848) 
-208.88143856876326

On 0.7:

julia> W2 = @evalpoly(0.1729150690306449, @evalpoly(1.2151064605177766, -27.0, -84.0, -56.0),
                                         -11.614665966268532,
                                         163.17740776843848) 
-191.5798454805012

vtjnash added a commit that referenced this issue Aug 16, 2017
previously, we were treating hygienic-scope as also introducing a new scope-block
that was wrong (and inconsistent with previous behavior)

also, we were failing to set the outermost flag when entering a
hygienic-scope block, further compounding the above error

fix #23239
vtjnash added a commit that referenced this issue Aug 17, 2017
previously, we were treating hygienic-scope as also introducing a new scope-block
that was wrong (and inconsistent with previous behavior)

also, we were failing to set the outermost flag when entering a
hygienic-scope block, further compounding the above error

fix #23239
@stevengj
Copy link
Member Author

@dlfivefifty, your case is also fixed by #23247.

@dlfivefifty
Copy link
Contributor

👍 FastGaussQuadrature.jl now passes all tests on 0.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:lowering Syntax lowering (compiler front end, 2nd stage) macros @macros
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants