这是indexloc提供的服务,不要输入任何密码
Skip to content

Added occurrence typing for private fields. #14

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

Merged
merged 3 commits into from
Feb 12, 2015

Conversation

takikawa
Copy link
Contributor

Adds occurrence typing for private fields. This has been implemented on an experimental branch for a while. The new thing is that it now comes with mutation checking in the class body to disable occurrence typing when necessary.

Code reviewers: my main concern is that the use of FieldPE path elements on the object of the accessor function is too hacky (see code in tc-envops.rkt). I can't think of any other way to implement it though.

@@ -16,6 +16,7 @@
(def-pathelem StructPE ([t Type?] [idx natural-number/c])
[#:frees (λ (f) (f t))]
[#:fold-rhs (*StructPE (type-rec-id t) idx)])
(def-pathelem FieldPE () [#:fold-rhs #:base])

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you let-alias an expression with this sort of path? ex: (let ([x exp]) ...) where the object of exp has a FieldPE in its path?

If so, I think a change similar to that in update would need to be added to
https://github.com/takikawa/typed-racket/blob/priv-field-occ-type/typed-racket-lib/typed-racket/types/path-type.rkt

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think if you do (let ([x f]) ...) where f is a private field name inside the class. I think you're right that I need to add a case there.

@takikawa
Copy link
Contributor Author

I think I've fixed this for let-aliasing now. Thanks for the feedback. It turned out that recent versions of plot actually depend on this let-aliasing to work correctly with this PR (which confused me for a while since the let only existed in the expansion of a match expression).

#:object
(make-Path (list (make-FieldPE)) getter-id))))
(-> (make-Univ) (or (and maybe-type (car maybe-type))
-Bottom)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When is this -Bottom?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there isn't a type annotation for the field, in which case we don't know how we can mutate the field so it's disallowed. (we can't necessarily just synthesize the field type first, because the field initial value expression may be a method call)

@samth
Copy link
Member

samth commented Feb 6, 2015

This is in pretty good shape, just a few comments.

#:filters -no-filter
#:object
(make-Path (list (make-FieldPE)) getter-id))))
(-> (make-Univ) (or (and maybe-type (car maybe-type))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use -Univ instead of allocating and hashing every time with (make-Univ).

This also tracks mutation of private fields to ensure that only
non-mutated field types are allowed to be refined.
Type aliases in internal definition contexts can affect
type printing outside of the context, which can cause
interference in unit tests.

This only seems to happen when running with the TR test
driver.
@takikawa takikawa force-pushed the priv-field-occ-type branch from b51c398 to 888893d Compare February 12, 2015 21:33
@takikawa takikawa merged commit 888893d into racket:master Feb 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants