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

Julep: Internal Properties #54

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Julep: Internal Properties #54

wants to merge 4 commits into from

Conversation

c42f
Copy link
Member

@c42f c42f commented Feb 8, 2019

Handy link to the document:
https://github.com/JuliaLang/Juleps/blob/cjf/internal-properties/InternalProperties.md

I started this over at JuliaLang/julia#30975, but keep wanting to edit the description. I figure it might be better to put it here.

This PR is an attempt to state the problem clearly, without settling on a particular solution. I think it would be useful to write down some potential solutions (maybe in the comments section), especially for the purposes of refining what the goals should be.

A concrete proposal can be added here or in a later PR.

@c42f
Copy link
Member Author

c42f commented Feb 8, 2019

Here's one flavor of possible solution, written with the access of data fields in mind (proposal 2 from JuliaLang/julia#30975)

Proposal 2: Module-local lowering of field access

Change the lowering of x.a to a module local shim function getprop(x, :a), which defaults to getproperty for all types, but automatically gets a method which calls getfield for every new type introduced into the module.

I think this does much better on several of the stated goals; it kind of "dissolves" goals 1-2, and does far better on goal 5, as it doesn't uglify the implementation details of a module.

Implementation sketch:

module Mod1

export Private, Public, use_field

# @private struct Private ...
struct Private
    a::Int
end

struct Public
    a::Int
end

# use_field(x) = x.a
use_field(x) = Mod1.getprop(x, :a)  # New lowering for x.a

## The following are auto generated
# Each module gets a default `getprop`
getprop(x, s) = getproperty(x, s) 
# Each type gets a method of the module-local `getprop`
getprop(x::Private, s) = getfield(x, s)
getprop(x::Public, s) = getfield(x, s) 
# Invocations of `@private struct ...` generate something like
Base.getproperty(x::Private, s::Symbol) = error("`Private.$x` is an internal field")

end


module Mod2
getprop(x, s) = getproperty(x, s)  # Auto generated

using ..Mod1

# use_field(x) = x.a
use_field(x) = Mod2.getprop(x, :a)

end

Mod1 is allowed to use the fields of types defined inside it with agreeable syntax:

julia> Mod1.use_field(Mod1.Public(1))
1
julia> Mod1.use_field(Mod1.Private(1))
1

Mod2 is not allowed to use the nice syntax to access the internal field of Mod1.Private. It would have to use getfield for that:

julia> Mod2.use_field(Mod1.Public(1))
1
julia> Mod2.use_field(Mod1.Private(1))
ERROR: `Private.Main.Mod1.Private(1)` is an internal field

What about cases where implementation details are shared between modules? For example, between several closely cooperating submodules within a project? This could be handled by explicitly importing getprop so that the method table is shared between modules.

Compatibility

Introducing the above is fairly compatible with the existing mostly getfield-based ecosystem, as only types marked with @private would disallow module-external access to the fields. However, it breaks packages which define getproperty and use it internally within the same module via the x.a syntax.

A transition plan during the 1.x timeframe could be to make this lowering opt-in on a per-module basis, with a compiler meta attached to the module AST. Then switch the lowering completely in 2.0.

@c42f
Copy link
Member Author

c42f commented Feb 8, 2019

Over on slack, @StefanKarpinski notes

While fields are one consideration, I wonder if we shouldn’t be thinking bigger and allow descriptions of what method calls are public versus private. Calling getproperty and setproperty! would just fall out as a special case of that.

I'm not sure I quite understand this comment but it seems like it might resolve the strange duplication of getprop/getproperty in the sketch above by allowing method matching to depend on the module from which it's called.

@mauro3
Copy link

mauro3 commented Feb 8, 2019

Good to see that you're thinking about this!

X-refs JuliaLang/julia#12064 and JuliaLang/julia#12069.

@c42f
Copy link
Member Author

c42f commented Feb 8, 2019

Thanks! There's also JuliaLang/julia#30204. I've added these to the document.

@c42f c42f force-pushed the cjf/internal-properties branch from 9d8e951 to 3b082ad Compare February 8, 2019 08:16
@c42f
Copy link
Member Author

c42f commented Jul 2, 2019

@vtjnash I just came across your PR JuliaLang/julia#22147 (and understood the content of your comment JuliaLang/julia#25750 (comment)) which has many similarities to what I was thinking about here. Nice!

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.

2 participants