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

support type annotations on global variables #43671

Merged
merged 39 commits into from
Feb 8, 2022
Merged

Conversation

simeonschaub
Copy link
Member

@simeonschaub simeonschaub commented Jan 5, 2022

This is an initial proposal for supporting typed globals based on #43455.
The syntax for this is either global x::T or just doing x::T = 1 in
global scope. It is even supported to add these annotations to globals
from inside functions. The type declaration will then be applied when
the method is defined and inside the function body conversion to the
specified type will happen automatically, similar to type annotations
for local variables.

This conversion will not be applied if the assignment is not done inside
the same scope of the type annotation however. This could potentially be
supported as well, but the problem is that this would mean any
assignment to a global variable - typed or not - would need to go
through a call to convert first, since lowering can't know if a given
binding already has a type annotation. This probably wouldn't be a good
thing for latency and sysimage size and would add an invalidation risk.

Any assignment to a global now adds a conversion step and a type
assertion, similar to how type annotations work inside local scopes.

It is allowed to refine a type annotation, but there will be a warning.
Widening the type or changing it to any other type that is not a subtype
of the previous one is an error, since that could cause cached compiled
code to become invalid.

replaces #43455
closes #964

@simeonschaub simeonschaub added compiler:lowering Syntax lowering (compiler front end, 2nd stage) needs docs Documentation for this change is required needs news A NEWS entry is required for this change compiler:inference Type inference labels Jan 5, 2022
@JeffBezanson
Copy link
Member

This conversion will not be applied if the assignment is not done inside
the same scope of the type annotation however. This could potentially be
supported as well

I think we will need to support this. Users would expect to write x::Int = 0 at the top level once, and have that apply everywhere. I know it's a bit annoying, which is why we haven't done this yet 😄 But assigning to globals should already be quite rare and slow, so the extra convert call is fine with me.

src/builtins.c Outdated Show resolved Hide resolved
@KristofferC
Copy link
Member

It is even supported to add these annotations to globals
from inside functions. The type declaration will then be applied when
the method is defined and inside the function body conversion to the
specified type will happen automatically, similar to type annotations
for local variables.

That seems like a quite weird feature to me. Was this implemented because it was easy or does it have some good use cases?

@simeonschaub
Copy link
Member Author

simeonschaub commented Jan 6, 2022

We could disallow adding these type annotations inside local scopes, but since we already allow defining new globals from local scopes, I think it's only consistent to allow declaring new typed ones as well. This was a bit of a special case before my last commit, since lowering would then use that annotation to add the automatic conversion. Now, this automatic conversion is always added on assignments to globals though, so there's not really much special handling except that we explicitly tell lowering to always set binding types from the toplevel via the toplevel-bufirst expression head.

@antoine-levitt
Copy link
Contributor

Very cool, great feature! Is this limited to literal types, or will it be possible to do T = Int32; x::T? If so, we could introduce a @typeconst macro (or even language keyword?) and have @typeconst x=2 lower to temp = 2; x::typeof(temp) = temp; that would get rid of the most common reason for the misuse of const (at least for me). (then, in a perfect world, it would be allowed to redefine the type of global variables, and finally have performant and flexible global variables...)

@simeonschaub
Copy link
Member Author

Is this limited to literal types, or will it be possible to do T = Int32; x::T?

Any valid top level expression is fine.

I'm not sure we need such a @typeconst macro in base, but that can always be discussed in a future issue/PR.

@simeonschaub simeonschaub reopened this Jan 7, 2022
@simeonschaub
Copy link
Member Author

/buildkite rerun failed

miguelraz and others added 4 commits January 8, 2022 10:47
new types should go at end of struct
This is an initial proposal for supporting typed globals based on #43455.
The syntax for this is either `global x::T` or just doing `x::T = 1` in
global scope. It is even supported to add these annotations to globals
from inside functions. The type declaration will then be applied when
the method is defined and inside the function body conversion to the
specified type will happen automatically, similar to type annotations
for local variables.

This conversion will not be applied if the assignment is not done inside
the same scope of the type annotation however. This could potentially be
supported as well, but the problem is that this would mean any
assignment to a global variable - typed or not - would need to go
through a call to `convert` first, since lowering can't know if a given
binding already has a type annotation. This probably wouldn't be a good
thing for latency and sysimage size and would add an invalidation risk.

It is allowed to refine a type annotation, but there will be a warning.
Widening the type or changing it to any other type that is not a subtype
of the previous one is an error, since that could cause cached compiled
code to become invalid.

replaces #43455
closes #964
simeonschaub added a commit that referenced this pull request Feb 15, 2022
aviatesk pushed a commit that referenced this pull request Feb 15, 2022
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request Feb 17, 2022
add type declarations to global bindings

replaces JuliaLang#43455
closes JuliaLang#964

Co-authored-by: Miguel Raz Guzmán Macedo <[email protected]>
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request Feb 17, 2022
simeonschaub added a commit that referenced this pull request Feb 17, 2022
aviatesk added a commit to JuliaDebug/JuliaInterpreter.jl that referenced this pull request Feb 18, 2022
- it seems like `:ccall` preserves may now include `SlotNumber` objects,
  and so we need to update `build_compiled_call!` accordingly
- JuliaLang/julia#43671 increased the number of
  statements of code that involves assignment to a global variable
simeonschaub added a commit that referenced this pull request Feb 18, 2022
aviatesk added a commit to JuliaDebug/JuliaInterpreter.jl that referenced this pull request Feb 19, 2022
- it seems like `:ccall` preserves may now include `SlotNumber` objects,
  and so we need to update `build_compiled_call!` accordingly
- JuliaLang/julia#43671 increased the number of
  statements of code that involves assignment to a global variable
aviatesk added a commit to JuliaDebug/JuliaInterpreter.jl that referenced this pull request Feb 19, 2022
- it seems like `:ccall` preserves may now include `SlotNumber` objects,
  and so we need to update `build_compiled_call!` accordingly
- JuliaLang/julia#43671 increased the number of
  statements of code that involves assignment to a global variable
KristofferC pushed a commit that referenced this pull request Feb 19, 2022
This got missed in #43671.

(cherry picked from commit 6c51d9e)
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
add type declarations to global bindings

replaces JuliaLang#43455
closes JuliaLang#964

Co-authored-by: Miguel Raz Guzmán Macedo <[email protected]>
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
staticfloat pushed a commit to JuliaCI/julia-buildkite-testing that referenced this pull request Mar 2, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
add type declarations to global bindings

replaces JuliaLang#43455
closes JuliaLang#964

Co-authored-by: Miguel Raz Guzmán Macedo <[email protected]>
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
StefanKarpinski pushed a commit that referenced this pull request May 6, 2024
…54379)

The performance tip from #43671 by @simeonschaub 

> If a global is known to always be of the same type, [the type should
be
annotated](https://docs.julialang.org/en/v1/manual/variables-and-scoping/#man-typed-globals).

was a bit unclear to me, because it does not apply to `const` globals
(whose type requires no annotation). This PR clarifies that point
lazarusA pushed a commit to lazarusA/julia that referenced this pull request Jul 12, 2024
…uliaLang#54379)

The performance tip from JuliaLang#43671 by @simeonschaub 

> If a global is known to always be of the same type, [the type should
be
annotated](https://docs.julialang.org/en/v1/manual/variables-and-scoping/#man-typed-globals).

was a bit unclear to me, because it does not apply to `const` globals
(whose type requires no annotation). This PR clarifies that point
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:inference Type inference compiler:lowering Syntax lowering (compiler front end, 2nd stage)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Type annotations in global scope
9 participants