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

give IR forms for defining types valid linear structure #33553

Merged
merged 1 commit into from
Apr 28, 2020

Conversation

JeffBezanson
Copy link
Sponsor Member

@JeffBezanson JeffBezanson commented Oct 14, 2019

The type-defining IR forms (struct_type etc.) are currently very special: they violate the usual linear structure rules for IR, and evaluate their arguments in a non-standard way intertwined with some global effects. This PR tries to clean that up, removing the special cases and hopefully eventually leading to fixing #33183.

The new type-defining process here is broken into three steps at the IR level:

  1. Create a new partially-initialized type object and bind it to a local variable.
  2. Evaluate the supertype expression and set it.
  3. Evaluate and set field types, fix up partial types.
  4. Assign the type globally, doing the special check for valid redefinitions.

Uses only function calls (unfortunately requiring several new builtins though).

fixes #35416

@JeffBezanson JeffBezanson added compiler:lowering Syntax lowering (compiler front end, 2nd stage) kind:minor change Marginal behavior change acceptable for a minor release labels Oct 14, 2019
@JeffBezanson
Copy link
Sponsor Member Author

It seems to me that if we break everything down into simpler forms, JuliaInterpreter can continue to work unmodified, which would certainly be a plus.

@JeffBezanson JeffBezanson added this to the 1.5 milestone Apr 27, 2020
@JeffBezanson JeffBezanson merged commit a645d7f into master Apr 28, 2020
@JeffBezanson JeffBezanson deleted the jb/typedefIR branch April 28, 2020 18:38
@timholy
Copy link
Sponsor Member

timholy commented May 1, 2020

Since you asked about JuliaInterpreter, the changes were relatively minor (though not entirely trivial to track down):

We'd noticed the former differences in lowering, and even had to handle them specially to make sure a performance optimization didn't trigger errors. So I'm supportive of this change.

@NHDaly
Copy link
Member

NHDaly commented May 3, 2020

Hooray! Thanks Jeff! Awesome. :)

@o314
Copy link
Contributor

o314 commented Oct 12, 2020

Currently, eg. in Julia v1.5.2 it still remains a mention to those expr in

    (define (check-top-level e)
      (define (head-to-text h)
        (case h
          ((abstract_type)  "\"abstract type\"")
          ((primitive_type) "\"primitive type\"")
          ((struct_type)    "\"struct\"")
          ((method)         "method definition")
          (else             (string "\"" h "\""))))

@ https://github.com/JuliaLang/julia/blob/v1.5.2/src/julia-syntax.scm#L3786-L3790

I don't know if it is voluntary (for backward compat or something else), or an omission

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:minor change Marginal behavior change acceptable for a minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug while typing recursive struct definition with UnionAll in type
4 participants