typeassert syntax deprecations

[vtjnash]

deprecations for #964 and #16071

this will let us reuse global x::T and x::T = 1 to mean a typed global instead of the current meaning of type-assert

it will also let us reuse x::T (in statement position) to always mean typeassert, instead of sometimes meaning local variable declaration (depending on various positional and context rules)

[tkelman]

will doing what this deprecation says and adding local x::T always mean the same for code that needs to still run on 0.4?

[vtjnash]

that's exactly the intent. this adds a deprecation to those codepaths, but doesn't change their behavior

[tkelman]

does this also deprecate this form, or did you just add local because you're deprecating the no-assignment form used for ret ?

[vtjnash]

the code just looked silly with only local on the ret declation. I didn't deprecate this form, since it's not clear to me that is decided.

[toivoh]

I thought the idea was that :: in an expression should always be a type assert, and :: in local/global should mean that you create a typed variable.

Why do you want x::T = 1 to create a typed global? Seems like it will be more confusing and be an additional gotcha when you move code between global and local scope?

[tkelman]

very hard to grep for this in packages, and the warning is kind of confusing when you do

julia> x::Float64 = 1.0

WARNING: deprecated syntax "global x::Float64".

[tkelman]

and from

julia> function foo3(y)
    z::Float64
    z+y
end

WARNING: deprecated syntax "z::Float64".
Use "local z::Float64" instead.

WARNING: deprecated syntax "global z::Float64".

the double warning is also confusing

[vtjnash]

the double warning is also confusing

no kidding, but that's a fairly accurate report on what this code used to mean. because you don't have an assignment to z, what looks like a local declaration (z::Float64) instead is actually parsed as a type-assert for the global variable z (and where both meanings are now deprecated).

very hard to grep for this in packages

yes, that's pretty much the entire point behind this deprecation

[vtjnash]

Why do you want x::T = 1 to create a typed global? Seems like it will be more confusing and be an additional gotcha when you move code between global and local scope?

I think it's more consistent to be either a syntax error or an implicit declaration. The current meaning of a type-assert on the old value seems least useful to me, although it is consistent with the current behavior for mutating assignments such as x::Int += 1. If it means a typed global, then it would be meaningful to write:

global x
x::T = 1

[tkelman]

the deprecation warning should say - the syntax used in the code is deprecated, and what to replace it with. right now it does neither, the warning includes keywords that the user code did not.

[vtjnash]

we don't have good tracking in the scheme code of what the user input, I can add that the user should use typeassert instead.

[toivoh]

@vtjnash: I think that a syntax error for x::T = 1 would be best and most consistent, since it alerts people who would expect either way of working that they should make their intentions clear.

Quoting @StefanKarpinski from #16071:

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 think that we should reduce the confusion to a minimum and only declare typed variables using local/global.

[tkelman]

the lack of line numbers for these warnings is really a bummer

(sorry about the repeat posts, phone being stupid)

[StefanKarpinski]

@vtjnash: I think we need to make a go/no go decision on this change today. Personally, I think that we should deprecate this, but I'm not sure that it's a great idea to rush the deprecation into a release right at the very end of the process. This would be a perfectly acceptable deprecation to do early in the next release cycle.

[tkelman]

preferably with line numbers if at all possible, so people aren't at a complete loss about what they need to fix

[vtjnash]

@tkelman the nice thing about error reporting features is that they can be added during the RC. the other nice thing about them is that they should not be added in the same PR as a feature decision.

[vtjnash]

I'm not sure that it's a great idea to rush the deprecation into a release right at the very end of the process

Can you clarify this? I don't think this'll have much of an impact on packages, since it's also not a common idiom in base. It's also not the same as adding a feature, since this is purely just deprecating old syntax by adding warnings whenever that codepath is active.

[StefanKarpinski]

Putting anything into a release right at the end is kind of sketchy.

[tkelman]

good point about separation of concerns, but with the current state of the error reporting, doing this now is a net negative. any deprecation will be noticed, base idioms don't really correlate with idioms used in packages.

[JeffBezanson]

I think it's ok to keep x::T = ... as-is, since the x there is clearly not in value position. The really important thing to deprecate is x::T in statement position where x is local.

I don't think we should have a deprecation warning at all for x::T where x is global, since that is currently a typeassert and can continue to be a typeassert in the future. IIUC, the construct whose behavior will change is global x::T, which is currently a typeassert but should be a declaration (to be consistent with local x::T). This will also fix the double-warning issue. When we see an actual global keyword (as part of the syntax global x::T), we should say something like "use global x; x::T instead; declaring types of globals is not yet supported but may be in the future".

[vtjnash]

Any opinion on whether x[]::T = ... should be deprecated? I've opted to leave it for now, since unlike the other syntax here, we wouldn't want it to later mean something different.

[JeffBezanson]

Yes, let's leave it for now to focus this change on the most urgent part.

[JeffBezanson]

I'll do NEWS and doc updates for this.