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

implement local const #5148

Open
bkamins opened this issue Dec 14, 2013 · 39 comments
Open

implement local const #5148

bkamins opened this issue Dec 14, 2013 · 39 comments
Assignees
Labels
compiler:lowering Syntax lowering (compiler front end, 2nd stage) kind:feature Indicates new feature / enhancement requests

Comments

@bkamins
Copy link
Member

bkamins commented Dec 14, 2013

I am wondering why code:

    function f()
      const x = "aaa"
      x = 4
      return x
    end
    println(f())

executes and prints 4 without any warning or error, while

    const x = "aaa"
    x = 4
    println(x)

raises:
ERROR: invalid redefinition of constant x

Similarly:

    function f()
      const x = "aaa"
      x = "bbb"
      return x
    end
    println(f())

prints bbb without warning, while:

    const x = "aaa"
    x = "bbb"
    println(x)

prints bbb, but with warning:
Warning: redefining constant x

I think behavior of constants should be consistent in global and function scopes.

My Julia version:

    Julia Version 0.2.0
    Commit 05c6461 (2013-11-16 23:44 UTC)
    Platform Info:
    System: Windows (x86_64-w64-mingw32)
    WORD_SIZE: 64
    BLAS: libopenblas (USE64BITINT DYNAMIC_ARCH NO_AFFINITY)
    LAPACK: libopenblas
    LIBM: libopenlibm
@ivarne
Copy link
Sponsor Member

ivarne commented Dec 14, 2013

I don't think that the const declaration is doing anything useful inside a function. Maybe users should be warned with a syntax error.

@bkamins
Copy link
Member Author

bkamins commented Dec 15, 2013

Currently the difference between local and const inside a function is that const forces you to assign a value to a variable.
At least consts should be explained more clearly in Julia Language Documentation.
Function was only one example. The same problem is also with other constructs introducing new scope, for instance:
for i in 1:5
const x = "abc"
x = i
println(x)
end

In fact current const behavior is the following:

  1. in global scope: fixes variable type, warns upon change of value (so in fact what consts in global scope do is to bind type to a variable);
  2. in other scopes: only forces assignment of a value on definition.

I agree with the opinion that non-global scope consts are currently misleading.

@quinnj
Copy link
Member

quinnj commented Aug 4, 2014

This seems like something that could be fairly confusing/unintuitive to a new user (indeed, I think I've seen several posts in julia-users where const is used inside scopes). Perhaps using const inside a scope should be equivalent to

function foo()
    const bar = 1
    return bar
end
# equivalent to
function foo()
    bar::Int = 1
    return bar
end

That way the user gets the same const behavior inside scopes.

@toivoh
Copy link
Contributor

toivoh commented Aug 4, 2014

I would really like to see true const semantics inside functions as well at
some point. Besides consistency, it makes it easier to write correct code
in some cases.

@quinnj
Copy link
Member

quinnj commented Aug 4, 2014

Yeah, I guess there are some subtleties in my proposal that would probably get us in trouble. Like the fact that :: is like inserting calls to convert within the scope body while const is more of a type-assert in global scope. Though I guess there are uses of :: that are purely type-assert, right? When it's used on assignment? I guess I'm realizing I may not fully understand all the subtle differences between const in global scope vs. :: as a type declaration in scope blocks vs. :: as a type assert with assignments.

@toivoh
Copy link
Contributor

toivoh commented Aug 5, 2014

Actually, I probably don't either. I was thinking that at least in local
scope, const should actually mean constant, but now I remember that there
has been talk about allowing to change global consts as long as they keep
their type.

@StefanKarpinski
Copy link
Sponsor Member

In global scope, you might want to reload some code and reassign a constant in the process – as long as this doesn't invalidate generated code, it's fine, so we allow it with a warning. I can't think of any good reason for to change a const binding in local scope, so the best option here would be to just enforce const in local scope, imo.

@toivoh
Copy link
Contributor

toivoh commented Aug 5, 2014

I agree.

@JeffBezanson
Copy link
Sponsor Member

Yes, changing the values of constants is not something to encourage :)

@JeffBezanson
Copy link
Sponsor Member

Although an interesting question is whether to enforce it statically or dynamically --- i.e. check that only one assignment actually occurs at run time.

@StefanKarpinski
Copy link
Sponsor Member

This is actually something we should have for 1.0 since otherwise code that declares something const but changes would break. In other words, going from ignoring const to respecting it is a breaking change. At a minimum, we should not allow const in local scope at all in 1.0 and then we can at least introduce a working version of this feature in 1.x without breaking anything.

@bkamins
Copy link
Member Author

bkamins commented Feb 14, 2018

Having const deprecated on local variables I have the following questions:

  • syntax const global x = 1 and also global const x = 1 are both allowed in global scope; they are equivalent to const x = 1 if I understand it correctly; yes? The question is do we still want to allow all three forms in global scope?
  • syntax const global x = 1 and also global const x = 1 are both allowed in local scope - they define (or update if it was existing and constant and the type is correct - possibly with a warning) a global const x; I think this is OK, but the question is if we want to allow both orders of const and global keywords?

Whatever is decided I think it should be documented in the Julia Manual.

@bkamins
Copy link
Member Author

bkamins commented Feb 14, 2018

Additionally we have the following behavior in global scope:

julia> local x = 1
1

julia> const local y = 1
┌ Warning: Deprecated syntax ``const` declaration on local variable`.
└ @ nothing none:0
1

Maybe this is exactly what we want, but I feel that deprecating local in global scope would be the cleanest approach (I understand that in global scope adding local has no effect). If we do not then local const x = 1 will become an error even if executed in global scope where using const is allowed.

@cossio
Copy link
Contributor

cossio commented May 29, 2018

@StefanKarpinski are there plans to resolve this before v0.7?

@StefanKarpinski
Copy link
Sponsor Member

It is already deprecated:

julia> function f()
           const x = 1
       end
┌ Warning: Deprecated syntax ``const` declaration on local variable` around REPL[20]:2.
└ @ nothing REPL[20]:2
f (generic function with 1 method)

@JeffBezanson
Copy link
Sponsor Member

JeffBezanson commented May 29, 2018

0.7 is basically done; certainly nothing further will be done about this in that timeframe.

@NightMachinery
Copy link

Any updates on this? I want to inline some functions (closures) that are generated by other functions, and this needs them to be declared constant.

function preCompute(input)
# do heavy computations
  @inline function fastFunc(x)
    # use the precomputed values and be fast
    ...
  end
  return fastFunc
end
...
let a=2
  f=preCompute(...) 
  [f(i) for i in 1:10^8] # needs const to be inlined
end

@yuyichao
Copy link
Contributor

and this needs them to be declared constant.

What makes you think that is the case?

@NightMachinery
Copy link

@yuyichao I have done tests:

using InteractiveUtils

function genF(x, y)
    @inline function f()
        if rand() <= 0.5
            return x
        else
            return y
        end
    end
    return f
end

const fOK = genF(1111111111, 222222222)
function r2()
    fOK()
end

let
    f = genF(333333, 7777777)
    function r()
        f()
    end


    println(@code_lowered r())
    println("----------")
    println(@code_typed optimize = false r())
    println("----------")
    println(@code_typed r())
    println("----------")
    println(@code_llvm r())

    println("*********************8")

    println(@code_lowered r2())
    println("----------")
    println(@code_typed r2())
    println("----------")
    println(@code_llvm r2())
end

Search in the output, you'll see 1111111111 in the output, but not 333333.

@yuyichao
Copy link
Contributor

There's no change in inlining. Adding a local const will make no difference here since what you are observing is an irrelevant difference due to GLOBAL constant. If you want to see the same output, just use const global which is still allowed. Otherwise, there's no difference when you actually use f() in the function.

julia> function g()
           f = genF(333333, 7777777)
           return f()
       end

julia> @code_typed g()

@NightMachinery
Copy link

@yuyichao I don't quite understand what you mean. The code is clearly not getting inlined in your example, or r() in my example. But it is getting inlined when we use global constants. And it is also clear that julia should be able to inline in these examples.
BTW, using const global seems disallowed:

ERROR: LoadError: syntax: `global const` declaration not allowed inside function around /Users/evar/Base/_Code/uni/stochastic/jo1/inlineloop.jl:25

And it is not what I need anyhow; I need that function to be a constant and optimized as such within the local scope. It is not going to be a global constant, since the generated function (in my real usecase) depends on input:

using DataStructures
function preP(p)
    # https://en.wikipedia.org/wiki/Alias_method
    n = length(p)
    u = p * n
    uover = MutableBinaryMaxHeap{Tuple{Rational{Int},Int}}()
    uunder = MutableBinaryMinHeap{Tuple{Rational{Int},Int}}()
    k = Array{Int}(undef, n)
    function pushu(i, u)
        if u < 1
            push!(uunder, (u, i))
        elseif u > 1
            push!(uover, (u, i))
        end
    end
    for (i, u) in enumerate(u)
        pushu(i, u)
    end
    while ! isempty(uunder)
        (uu, iu) = pop!(uunder)
        if isempty(uover)
            k[iu] = -1 # Mark as invalid. We can also use a dummy default value.
            continue
        end
        (uo, io) = pop!(uover)
        k[iu] = io
        pushu(io, uo - (1 - uu))
    end

    P = let n = n, u = u, k = k
        @inline function x()
            i = rand(1:n)
            @inbounds if rand() <= u[i]
                return i
            else
                return k[i]
            end
        end
    end
    println("Precomputed alias tables for $p")
    return P
end

p = [1 // 10, 3 // 10, 4 // 10, 36 // 1000, 98 // 1000, 5 // 1000]
push!(p, 1 - sum(p))
@assert sum(p) == 1
const P = preP(p)

I want this P function to get inlined. Currently I can only do that with a global constant, and something like this is not possible:

function getDistributionOfP(p)
  local const P = preP(p)
  return prop(freqtable([P() for i in 1:10^8])) # inlining P not currently possible
end

@yuyichao
Copy link
Contributor

I don't quite understand what you mean. The code is clearly not getting inlined in your example, or r() in my example. But it is getting inlined when we use global constants. And it is also clear that julia should be able to inline in these examples.

It is inlined. What I said is basically that your interpretation of the output is wrong.

BTW, using const global seems disallowed:

Using it in let is allowed. And that's basically the only case this makes sense.

And it is not what I need anyhow; I need that function to be a constant and optimized as such within the local scope.

By all mean, it IS a local constant. It is a local variable that is never changed and the compiler had no problem figuring that out at all. In another word, allowing local constant won't make any difference for code that doesn't error when adding it.

There are indeed additional optimizations that can be done on GLOBAL contants, but,

  1. Those aren't something that you can get with a "local" constant, i.e.

    what you are observing is an irrelevant difference due to GLOBAL constant

  2. It doesn't make much difference if you are just using the function locally, i.e.

    Otherwise, there's no difference when you actually use f() in the function.

  3. You can only take advantage of it unless you are OK with a GLOBAL constant, i.e.

    If you want to see the same output, just use const global which is still allowed

Please ask any further questions on discourse.

@StefanKarpinski StefanKarpinski changed the title const does nothing when used with local variables implement local const Dec 1, 2022
@StefanKarpinski StefanKarpinski added the kind:feature Indicates new feature / enhancement requests label Dec 1, 2022
@StefanKarpinski StefanKarpinski removed this from the 2.0 milestone Dec 1, 2022
@StefanKarpinski
Copy link
Sponsor Member

I thought this was recorded here, but the main issue preventing this from being implemented is deciding what rule to use for local const. In global scope, constness is enforced dynamically. In local scope, however, we'd want a statically decidable rule. The question is what it should be. One simple option would be to only allow a single syntactic assignment, i.e. whereas this is allowed in global scope it would not be allowed in local scope:

if true
    const x = 1
else
    const x = 2
end

This still wouldn't guarantee that assignment occurs on every possible code path, however, as you could still write this:

if true
    const x = 1
end

In such a case x could still be unassigned. Perhaps that's acceptable.

@bkamins
Copy link
Member Author

bkamins commented Dec 1, 2022

I am not parser expert to say for sure what is easily/safely doable, but what I would feel that a safe and acceptable rule would be that
const is assigned only once and that const can be only first statements in a scope when a variable is introduced (so that we are sure that it is assigned in this scope).

In general I see that care needs to be also taken when combining const and local:

function f1() # this function should be OK
    for i in 1:10
        const x = i
        # do something with x
    end
end

function f2() # this function should error at compile time, although there is only one syntactic assignment here
    local x
    for i in 1:10
        const x = i
        # do something with x
    end
end

@uniment
Copy link

uniment commented Dec 1, 2022

In such a case x could still be unassigned. Perhaps that's acceptable.

At first glance this seems acceptable; if I forget to declare any other variable in a code path, I will run into errors anyway---const or not.

we'd want a statically decidable rule

My knee-jerk reaction is to allow only a single syntactic assignment, but I can see it being desired to allow the constant to take on a different value depending on the function's arguments or global environment. So I'll take a stab at satisfying this before settling on my instinct.

Perhaps, at most one syntactic assignment per syntactic code path, if that's meaningful and possible? Namely, if there is more than one assignment to a certain const in a local scope, they must be in syntactically mutually-exclusive code paths (e.g. separated by if...else or if...elseif)?

@StefanKarpinski
Copy link
Sponsor Member

Analysis of code paths is pretty complex and generally something we don't want to bring into these rules. You have to figure out what the basic blocks are and what the dominance graph of the blocks is... it's heavy.

One complication is that method definitions like f() = ... get lowered to something like

if !@isdefined(f)
    const f = make_function(:f)
end
add_method(f, :(...))

If you have a series of these in local scope, then you'd get multiple const assignments in a row. Only the first one will actually happen, but you can see the issue. I think @JeffBezanson might need to chime in on what's possible.

@vtjnash
Copy link
Sponsor Member

vtjnash commented Dec 1, 2022

I wonder if we should try to make this existing global expression either more syntactic or more dynamic than it is now. Right now, this:

if true
    const x = 1
end

turns into

global x
if true
    const x # or, hypothetically, setconst!(@__MODULE__, :x)
    setglobal!(@__MODULE__, :x, 1)
end

which leads to several odd intermediate states. I feel it would be preferable if this instead became:

if true
    setglobal!(@__MODULE__, :x, 1, #=const=#true)
end

which simultaneously would define a new const global :x and set its value to 1, or would error if the user had previously referred to x in any way (including testing @isdefined x)

@uniment
Copy link

uniment commented Dec 1, 2022

I suppose a counter-argument to the idea of allowing branch-dependent const declarations in a given scope is that it’s easy enough to specify:

const my_const = if … else … end

or using the ternary ? : syntax.

@LilithHafner
Copy link
Member

How much performance to we loose if we do this check dynamically (i.e. the same way as global consts)?

@uniment
Copy link

uniment commented Dec 4, 2022

Learning more about this issue is making me rofl

function always defined despite code path never executing from here:

julia> function fn_generator()
           if true
               f() = 0
           else
               f(x, y) = x+y
           end
       end
fn_generator (generic function with 1 method)

julia> name = fn_generator()
(::var"#f#3") (generic function with 2 methods)

julia> name(1,2) # 3??
3

function never defined despite code path always executing from here

julia> function fn_generator2()
           if false
               f() = 0
           else
               f(x, y) = x+y
           end
       end
fn_generator2 (generic function with 1 method)

julia> fn_generator2()
ERROR: UndefVarError: `f` not defined

the natural extension:

julia> function fn_generator3()
                  if rand(Bool)
                      f() = 0
                  else
                      f(x, y) = x+y
                  end
              end
fn_generator3 (generic function with 1 method)

julia> fn_generator3()
(::var"#f#4") (generic function with 2 methods)

julia> fn_generator3()
ERROR: UndefVarError: f not defined

julia> fn_generator3()
(::var"#f#4") (generic function with 2 methods)

julia> fn_generator3()
(::var"#f#4") (generic function with 2 methods)

julia> fn_generator3()
ERROR: UndefVarError: f not defined

function not defined despite code path executing from here

julia> @enum Case A B

julia> g = function (case, x)
           if case == A
               lambda(x) = 2x
           elseif case == B
               lambda(x) = x^2
           end
           lambda(x)
       end
WARNING: Method definition lambda(Any) in module Main at REPL[5]:3 overwritten at REPL[5]:5.
#4 (generic function with 1 method)

julia> g(A, 5) #  25 .. wait, what?
25

julia> g(B, 5)
ERROR: UndefVarError: `lambda` not defined

Thankfully it offers a warning, but it should be an error.

function defined only in code path where it was not defined; modified version of this

julia> function h(a)
         if a
           f() = 2
         else
           f() = 3
           f(x) = 4
         end
         return f
       end
WARNING: Method definition f() in module Main at REPL[5]:3 overwritten at REPL[5]:5.
h (generic function with 1 method)

julia> h(true)()
3

julia> h(true)(1)
4

julia> h(false)(1) # hahahaha
ERROR: UndefVarError: f not defined

Placing named function declarations inside loops

julia> function f1()
           g() = 1
           for i=1:3
               g() = @show(i); g() # reassigns `g` at parent scope for no good reason, where `i` is undefined
           end
           g
       end
WARNING: Method definition g() in module Main at REPL[33]:2 overwritten at REPL[33]:4.
f1 (generic function with 1 method)

julia> f1()()
ERROR: UndefVarError: i not defined

julia> i=3
3

julia> function f1()
           i=5; g() = 1
           for i=1:3
               g() = @show(i); g() # ok so we've defined `i` at local scope, parent scope, and global
           end
           g
       end
WARNING: Method definition g() in module Main at REPL[126]:2 overwritten at REPL[126]:4.
f1 (generic function with 1 method)

julia> f1()() # where is `i` *not* defined!?
ERROR: UndefVarError: i not defined

julia> function f1()
           g() = 1
           for i=1:3
               local g() = @show(i); g() # doesn't reassign `g` at parent scope, works as expected
           end
           g
       end
f1 (generic function with 1 method)

julia> f1()()
i = 1
i = 2
i = 3
1

julia> function f1()
           for i=1:3
               g() = @show(i); g() # doesn't assign `g` at parent scope; works as expected
           end
           g
       end
f1 (generic function with 1 method)

julia> f1()()
i = 1
i = 2
i = 3
ERROR: MethodError: no method matching g()

julia> function f1()
           g = ()->1
           for i=1:3
               g = ()->@show(i); g() # this is what you wanted anyway, if you wanted to reassign `g` at parent scope
           end
           g
       end
f1 (generic function with 1 method)

julia> f1()()
i = 1
i = 2
i = 3
i = 3
3

If someone's using this design pattern, of declaring named functions within conditional branches in local scopes, and hasn't gotten properly buggered by it yet, they will—it's a time bomb.

I am now much more strongly in support of outright disallowing const assignments nested in any conditionally-evaluated code within any given local scope (unless the const is enclosed in a scope more local than the conditional), as any existing notion of a local constant is already quite broken by attempting to be more accommodating. 😅 It also seems wise that any such local const declaration must behave as local const, strictly local to the most-local scope; there's no reason for a for loop or let statement to attempt to reassign a const declared at its parent scope. Of course, this behavior would extend to anything else that should be a local constant, namely named functions as proposed here.

For conditionally-evaluated code, we already have regular variables and anonymous functions; we don't need to be so abusive toward our constants 😝.

const can be only first statements in a scope when a variable is introduced

I don't think we need to be that restrictive though; it can be useful to precompute some values first before assigning the constant.

So by this proposal:

let # valid
    const b = if x 1 else 2 end
end

let # valid
    if x a=1 else a=2 end
    const b=a
end

let # error
    if x
        const b = 1
    else
        const b = 2
    end
end

let b = 2 # error
    if x
        const b = 1
    end
end

let # error
    x && (const b = 1)
end

let # valid 
    const b = x ? 1 : 2
end

let # error
    x ? (const b = 1) : (const b = 2)
end

let # error
    const b = 1
    b = 2
end

let # error
    b = 1
    const b = 2
end

let # valid, but `c` is local to the innermost `let` statements and so is not used
    if x
        let; const c = 1 end
    else
        let; const c = 2 end
    end
end

let # valid, but `const b` is local and does not over-write outer scope `b`
    b = 1
    if x
        let; const b = 1 end
    else
        let; const b = 2 end
    end
end

let # valid, but `b` is local and does not over-write outer scope `const b`
    const b = 1
    if x
        let; b = 1 end
    else
        let; b = 2 end
    end
end

@LilithHafner
Copy link
Member

Some of those examples are really bad.

@jariji
Copy link
Contributor

jariji commented Jan 25, 2024

I keep making mistakes that local const could have prevented so I would really like this feature.

#5148 (comment)

One simple option would be to only allow a single syntactic assignment

This rule seems fine, and can always be loosened later if desired.

@StefanKarpinski
Copy link
Sponsor Member

One issue with that rule is that it's fairly common to have local functions with multiple methods, which is naturally lowered to multiple assignments.

@uniment
Copy link

uniment commented Jan 30, 2024

Multiple local method declarations are not currently lowered to multiple assignments.

Instead they become multiple :method expressions for a single ["anonymous"] globally-defined struct, and the function's local symbol (which is currently not a constant, unfortunately) is assigned only once to an instance of it.

(For example, run Meta.@lower let; f() = 0; f(x, y) = x+y end)

@uniment
Copy link

uniment commented Feb 10, 2024

@StefanKarpinski said a while ago,

Analysis of code paths is pretty complex and generally something we don't want to bring into these rules. You have to figure out what the basic blocks are and what the dominance graph of the blocks is... it's heavy.

I think there's a simple solution: place a mild syntax constraint on the user.

I propose this rule: Whenever a local constant is declared within a conditional, the conditional must declare and assign it exactly once. A conditional is considered to declare and assign a constant exactly once if (1) the conditional has an else default branch, and (2) within every branch of the conditional the same constant is syntactically declared and assigned exactly once. Note that this rule works recursively for nested conditionals.

Examples:

Valid syntax:

let
    if p
        if q;  const a = 1
        else   const a = 2
        end
    elseif q;  const a = 3
    else       const a = 4
    end
end

Invalid syntax (local constant declared in conditional without default else branch):

let
    if p
        if q;   const a = 1
        else    const a = 2
        end
    elseif q;   const a = 3
    elseif !q;  const a = 4  # <- 
    end
end

Invalid syntax (local constant declared in conditional but not declared in every branch)

let
    if p
        if q;  const a = 1
        else   const a = 2
        end
    elseif q;  const a = 3
    else       println("help")  # <- 
    end
end

This rule is reasonably easy to communicate to users, and is likely to be something they'd already follow anyway (and if not, they should).

This would be a breaking change if it was implemented at global scope, but you might struggle to find examples that actually break if you did. (I'm not suggesting to implement this at global scope, I just think this would cover like 99.9% of use cases.)

@vtjnash
Copy link
Sponsor Member

vtjnash commented Feb 10, 2024

That seems identical to what #5148 (comment) said, but with somewhat less technical accuracy on how that would be implemented

@uniment
Copy link

uniment commented Feb 10, 2024

If you prefer, I can offer some untested pseudo-code to explain what I mean. It's just building up counts, comparing them to each other, and comparing them to 1:

# dictionary addition routine (to build up histogram):
define add!(a::Dict, b::Dict):
    for k in keys(b)
        a[k] = get(a, k, 0) + b[k]
    return a

# routine to check that constants are syntactically declared exactly once within a particular conditional:
define check(expr):
    if expr is a new nested local scope, return empty Dict()    # skip nested local scopes
    if expr is a const declaration, then: 
        return Dict(varname => 1)                               # base case: one const declaration
    if expr is a conditional, then:
        assert expr must have an else branch                    # condition must have a default branch
        firstcounts = check(first branch)
        assert all(isequal(1), values(firstcounts))             # check that const counts all equal 1
        assert all(isequal(firstcounts), map(check, branches))  # all branches must have equal const counts
        return firstcounts                                      # on success, return const counts
    if expr has subexpressions, then:
        return mapreduce(add!, check, subexpressions)            # add up all const counts across subexpressions
    return empty Dict()                                         # otherwise expression contains no const declaration

I'm not showing any reassignment check here, as I'm sure that's a done deal already

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) kind:feature Indicates new feature / enhancement requests
Projects
None yet
Development

No branches or pull requests