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

restrict typed variable declaration to local x::T and global x::T #16071

Closed
StefanKarpinski opened this issue Apr 27, 2016 · 18 comments
Closed
Assignees
Labels
compiler:lowering Syntax lowering (compiler front end, 2nd stage) good first issue Indicates a good issue for first-time contributors to Julia help wanted Indicates that a maintainer wants help on an issue or pull request speculative Whether the change will be implemented is speculative

Comments

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Apr 27, 2016

It's confusing that x::T is an assertion in some contexts and a declaration that x is a local variable of type T in other contexts. I propose we ultimately require a declaration keyword for the typed variable declaration usage, i.e. local x::T or global x::T. The first step is to deprecate the keywordless syntax, which should be done as soon as possible if we're going to do this.

@StefanKarpinski StefanKarpinski added speculative Whether the change will be implemented is speculative help wanted Indicates that a maintainer wants help on an issue or pull request parser Language parsing and surface syntax good first issue Indicates a good issue for first-time contributors to Julia labels Apr 27, 2016
@yuyichao
Copy link
Contributor

Does this mean after the deprecation period we can make x::T in statement position always mean typeassert? That'll be nice since AFAIK we you can only do that with Base.typeassert(x, T) now.

@StefanKarpinski
Copy link
Member Author

Yes, that is what it would mean.

@JeffBezanson JeffBezanson added compiler:lowering Syntax lowering (compiler front end, 2nd stage) and removed parser Language parsing and surface syntax labels Apr 29, 2016
@JeffBezanson
Copy link
Member

Seems like a good change. What should x::T = 0 mean?

@vtjnash
Copy link
Member

vtjnash commented Apr 29, 2016

I think that can still be an implicit local. Unless we want to enforce that the declaration must dominate the usages? For example, what does the following mean:

function f(x)
         if x
           T = Int
           y::(println("HI");T) = 2.0
         else
           y = 3.0
         end
         y
       end
f(false)

(answer: it prints "HI" twice then throws "UndefVarError: T not defined")

@StefanKarpinski StefanKarpinski changed the title restrict typed variable declaration syntax to local x::T and global x::T restrict typed variable declaration to local x::T and global x::T Apr 29, 2016
@toivoh
Copy link
Contributor

toivoh commented Apr 30, 2016

I think we should probably go all in and make type assertions on the left hand side of an assignment be an error.

Or possibly a type assertion on the old value, but that seems bound to be confusing. Anyway, consider x[5]::Int = 1, that's a type assertion on the lhs that needs to be treated in another way than as a variable declaration.

@StefanKarpinski
Copy link
Member Author

What should x::T = 0 mean?

Syntax error suggesting local x::T = 0 instead?

@StefanKarpinski
Copy link
Member Author

Unless we want to enforce that the declaration must dominate the usages?

I think this is a good idea. That example should be required to be written as:

function f(x)
    local y::Int
    if x
        y = 2.0
    else
        y = 3.0
    end
    return y
end

@Tetralux
Copy link
Contributor

local is implicit isn't it? Why would specifying a type for the variable require you specify that?

I would rather remove ::T as a shortcut for type assertions, and instead have the programmer write typeassert(x, T), or---perhaps more clearly, assert(typeof(x) === T).

@jamesonquinn
Copy link
Contributor

The above PR is probably not the final word; it uses peek after processing a :: expression to see if the following operator is an assignment, which means it catches a simple a::Int = 3 but misses things like (a::Int) = 3 or a::Int, b = 3,4. To work more generally, it would have to look backwards rather than forwards; that is, pick apart the LHS expression when it encounters an assignment. I believe that's a matter of recursing down through tuples looking at the car of each item, but I'm not certain, and at any rate that would be more work. I wanted to keep my first PR simple so people could tell me if I'm doing it wrong.

@vtjnash
Copy link
Member

vtjnash commented Jul 14, 2016

I'm strongly in favor of this. Just tracked down a latent bug in the numbers test to

for T in (Int32,Int64)
    i::T, j::T = 1, 2
end

sometimes meaning (and doing) something completely different for i and j in that expression: :: means either of a type-assert or a local variable declaration in either usage (yikes!).

In observation, since the dates test used j as a global, that expression is a declaration of a local variable i with type T and assignment local i = convert(2, T)::T, vs. a typeassert that j::T followed by an assignment of j = 2.

@vtjnash
Copy link
Member

vtjnash commented Jul 14, 2016

can I nominate this deprecation in 0.5 (and volunteer myself to work on it now)? (or at least the non-breaking part that in x::Foo = 1, the :: will never mean typeassert, which is the deprecation for #964)

@tkelman
Copy link
Contributor

tkelman commented Jul 14, 2016

If you want to finish this before we feature freeze I'd be okay with it. I'm not okay with putting a patch I haven't seen yet on the milestone right now.

@vtjnash vtjnash self-assigned this Jul 15, 2016
@StefanKarpinski
Copy link
Member Author

sometimes meaning (and doing) something completely different for i and j in that expression: :: means either of a type-assert or a local variable declaration in either usage (yikes!).

Can you explain this a little more, @vtjnash?

vtjnash added a commit that referenced this issue Jul 15, 2016
…local variable

and fix a bug in env.jl where a typeassert was intended

ref #16071
@vtjnash
Copy link
Member

vtjnash commented Jul 15, 2016

Sometimes x::T = 1 could mean: the local variable x has type T and assign it the value convert(T, 1); other times, it could mean: typeassert the global variable x was of type T, then assign it the value 1. Anyways, PR submitted to add deprecation warnings to the context-dependent usages.

@cstjean
Copy link
Contributor

cstjean commented Jul 15, 2016

What about type annotations in loops and function arguments? In 0.4, the former is a type declaration, but the latter is not (function foo(x::Int) x="hello"; x*x end; foo(2) is valid).

@vtjnash
Copy link
Member

vtjnash commented Jul 16, 2016

Those already seem consistent to me (and unchanged from 0.4). Am I missing something?

julia> for x::Int = 1:10; x = 0.0; end
# declares that x has type Int

julia> function foo(x::Int) x="hello"; x*x end
# "asserts" that the first argument was passed an Int

@cstjean
Copy link
Contributor

cstjean commented Jul 16, 2016

You're right, I wasn't thinking of the loop as an assignment, but it makes perfect sense.

vtjnash added a commit that referenced this issue Jul 18, 2016
…local variable

and fix a bug in env.jl where a typeassert was intended

ref #16071
vtjnash added a commit that referenced this issue Jul 21, 2016
…local variable

and fix a bug in env.jl where a typeassert was intended

ref #16071
JeffBezanson pushed a commit that referenced this issue Jul 21, 2016
deprecate `x::T` as type declaration syntax, ref #16071

deprecate `x::T = 0` where x is global and `global x::T` meaning typeassert, ref #964

also fix a bug where a typeassert was considered to be effect-free,
causing it to be quasi-converted into a local variable declaration

fix a bug in env.jl where a typeassert was intended
@JeffBezanson
Copy link
Member

Fixed by #17445

mfasi pushed a commit to mfasi/julia that referenced this issue Sep 5, 2016
deprecate `x::T` as type declaration syntax, ref JuliaLang#16071

deprecate `x::T = 0` where x is global and `global x::T` meaning typeassert, ref JuliaLang#964

also fix a bug where a typeassert was considered to be effect-free,
causing it to be quasi-converted into a local variable declaration

fix a bug in env.jl where a typeassert was intended
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) good first issue Indicates a good issue for first-time contributors to Julia help wanted Indicates that a maintainer wants help on an issue or pull request speculative Whether the change will be implemented is speculative
Projects
None yet
Development

No branches or pull requests

9 participants