Access control puzzle

Hi all,

In the below,

  • warning(3) means -swift-version 3 mode produces a warning.
  • error(4) means -swift-version 4 produces an error.

According to SE-0025, both x1's and y2's should be private since they're inside a private type. I would not expect to see any errors or warnings here. Or if that's wrong, I would expect the x3's and x4's to produce a warning/error too.

The missing warning in the second extension example in Swift 3 mode is a puzzle too, but I'm less concerned about that since it's just a warning and it's Swift 3 mode.

What should the expected behavior be?

struct Container {
  private struct PrivateStruct1 {
    private struct VeryPrivate1 {}
    private typealias VeryPrivate2 = Int

    let x1 = VeryPrivate1() // warning(3) error(4)
    let y2 = VeryPrivate2() // warning(3) error(4)

    fileprivate struct VeryFilePrivate1 {}
    fileprivate typealias VeryFilePrivate2 = Int

    let x3 = VeryFilePrivate1() // ok
    let y4 = VeryFilePrivate2() // ok
  }
}

extension Container {
  private struct PrivateStruct2 {
    private struct VeryPrivate1 {}
    private typealias VeryPrivate2 = Int

    let x1 = VeryPrivate1() // error(4)
    let y2 = VeryPrivate2() // warning(3) error(4)

    fileprivate struct VeryFilePrivate1 {}
    fileprivate typealias VeryFilePrivate2 = Int

    let x3 = VeryFilePrivate1() // ok
    let y4 = VeryFilePrivate2() // ok
  }
}

What are you trying to do with this type in container type?

I'm pretty sure the answer is "nothing": Afaik, Slava is to busy keeping Swift running to actually use it himself ;-)

I can't back my answer with official documents, but imho it would be sensible to allow the typealias case without complaining:
This would allow users to specify a type that's used in several locations which can be changed easily.
For example, you could require different numeric types depending on target platform or other flags. It's not needed to expose the alias in such a situation, so it's preferable to keep it private.

The other case seems to be not very complicated (in this context: the compiler can easily deduce the correct access level), so I think a warning (with a fixit) is better than an error that interrupts compilation.

1 Like

I am sorry but I never understood why Types can be added inside an extension when stored properties are not allowed.

I would expect explicit private Types inside extensions to not be allowed. That was not the spirit of se-25 which seeked to hide implemention details like methods.

I would personally expect x3 and y4 to be disallowed. My mental model is that the access control of the type is separate to its members; you should be able to model the members of a type as if that enclosing type were public, and then independently decide the access level of the enclosing type later.

A type inside an extension does not present the same implementation difficulties as stored properties inside an extension.

2 Likes

Why? What's so bad about this pattern?

protocol ProtoA {
  var b: ProtoB { get }
}
extension MyType : ProtoA {
  private struct HiddenThing : ProtoB {
    init(_ value: MyType) { ... }
    ...
  }

  var b: ProtoB {
     return HiddenThing(self)
  }
}

That private struct can never escape that scope. How useful is that? If a super private implementation needs its own super private types then I think it probably deserves its own file or at least its own private type Instead of bloating an extension scope. I don’t buy it.

Off topic discussion (unfold to read).

And then here we go again and lose the private access control. (We do not always want to expose everything for re-use.) You can put it on the top level of the file but then you may need to rearchitect your access control and start using fileprivate more often because the otherwise nested type won't get access to the previously parent types private members.

What are your concerns, readability, ergonomics or anything else?

fileprivate struct A {
  let string: String
  init(_ value : MyType) {
    // 'string' is inaccessible due to 'private' protection level
    string = value.string
  }
}

struct MyType {
  // Must be `fileprivate` if we want expose access to `A`
  private let string = "swift"
}

extension MyType {
  private struct B {
    let string: String
    init(_ value : MyType) {
      // just fine
      string = value.string
    }
  }
}

To me this looks like you want inheritance. Just use classes and you get inheritance. Is this again a principal thing that speaks against using the tools provided by the language?

This thread isn't about "what else you can and should use". I'm not sure where this inheritance smell just came from, there is nothing about inheritance in there at all. Slava is trying to solve an important issue in this thread while I asked Chéyo Jiménez about his concerns of one particular pattern involving access control and nested types.

But your pattern is trying to do inheritance x_X

And yes - pointing out other ways to reach a goal that work are a solution. Sometimes a wanted pattern is simply wrong in the context of the used language/framework.

Off topic discussion (unfold to read).

Excuse me? Just because I re-used one type member in a nested type of the parent type to show case difference in access control of nested types vs. file-bound types != inheritance.

I already answered that in my previous reply, if you want to use bugged version of the language and prevent experts solving issues because you think 'everyone should just use other functionality to workaround a problem', then you're in the wrong thread.

I'm just promoting using the language as it is designed not to fight it's design. Using the right feature for a task is not using workarounds.

That's paradoxical since it's you who decides what 'right' means and not the 'design' which no one is fighting in this thread against. Asking someone why that person does not like a particular pattern should not imply that there is a better pattern that one should use instead. It's about understanding the rationale for this thinking.

Nesting types inside a types body or in an extension adds more complexity but also more flexibility in terms of access control which we can reason about when we're building strong and safe API's. Keep in mind that complexity is always tied together to flexibility. It wasn't my intention to promote any pattern over the other. I just wanted to know the reasons why someone is thinking explicit private types inside an extensions were considered as code smell (which I personally may or may not agree with).

According to my reading of SE-0025, both x1 and y2 are technically internal but effectively private. You should be able to build the type PrivateStruct1 as private but set up its access control as if you plan to make it public later.

But this clause:

The type of a member can reference only declarations that are accessible wherever the member is accessible.

And the relevant examples:

... this code as illegal:

struct Outer {
 private struct Inner {
   private typealias Value = Int
   var value: Value
 }
}

Seems to suggest that the ok/warning/error for a declaration should be evaluated as:

  • If there is no explicit access control level specified for a member, evaluate that member as if it is declared as internal
  • If there is an explicit ac level specified, evaluate it as written

I think the intent here is to avoid surprise errors later when you come by and open up access to this type, only to be ambushed by errors for all the member declarations that should have been private, but were only effectively private by implied default.

Edit: Forgot to actually state my opinion:

So Based on that, I think I'd expect the warnings/errors in both cases (x1, y2, x3, y4), since evaluating them as internal would produce those errors, as the types of each are less visible than internal.

My concern is not about super private but about nesting types inside extensions. The examples shown can easily be written with the nested types declared in the type itself. Do you have any examples of code in the wild that uses the pattern?

All the nesting of types I’ve seen in other people’s code always have the same visibility as the type being extended.

Off topic discussion (unfold to read).

That is correct, but why should we force that particular style? For instance you cannot even force that if you don't own the type. If we would start to do so it will be a breaking change.

Not in the wild but I found at least one in my library. I can theoretically move out outside the extension but I like to keep the custom coding key types near by and the keys do not need to be exposed at all.

extension CharacteristicCache : Codable {
  ///
  private struct Key : CodingKey {
    ///
    let stringValue: String

    ///
    let intValue: Int?

    ///
    init?(stringValue: String) {
      self.stringValue = stringValue
      self.intValue = Int(stringValue)
    }

    ///
    init?(intValue: Int) {
      self.stringValue = "\(intValue)"
      self.intValue = intValue
    }
  }

  ///
  public convenience init(from decoder: Decoder) throws {
    let container = try decoder.container(keyedBy: Key.self)
    let keysAndValues: [(String, Data)] = try container.allKeys.map {
      ($0.stringValue, try container.decode(Data.self, forKey: $0))
    }
    self.init(storage: [String: Data](uniqueKeysWithValues: keysAndValues))
  }

  /// Transforms the internal storage into a minial representation that can be
  /// easily archived and restored later when needed.
  public func encode(to encoder: Encoder) throws {
    var container = encoder.container(keyedBy: Key.self)
    try storage.lazy
      .filter { $0.value.shouldBeArchived }
      .map { (key: $0.key, value: $0.value.data) }
      .forEach {
        if let codingKey = Key(stringValue: $0.key) {
          try container.encode($0.value, forKey: codingKey)
        }
      }
  }
}

Edit: By super private you mean Swift 3 or Swift 4 private? I'm not against banning that functionality, but if we ever decide to do so, I'd like to know technical reasons other then it makes things complex. Since nothing is really trivial I personally consider that argument as worthless. ;-)

The missing warning in Swift 3 is a mystery. But otherwise, the behavior is correct because not all private is equivalent: it depends on the scope to which the private applies.

  • Container.PrivateStruct1.VeryPrivate1 is visible only within the containing type Container.PrivateStruct1
  • Container.PrivateStruct1.x1 is notionally internal and therefore bounded by the private visibility of Container.PrivateStruct1--i.e., it is visible within the scope of its containing type Container (and, since Swift 4, same-file extensions of Container) [*]
  • Therefore, the warning/error is correct that the property x1 must be declared private because it uses a private type: PrivateStruct1 is not visible everywhere that x1 is visible.

The same issue does not apply to the fileprivate types, because they are visible everywhere (?and more) that the variables x3 and x4 are visible.

Put another way:

/*
  VISIBLE IN THIS SCOPE:
  Container
  
  INVISIBLE IN THIS SCOPE:
  Everything else
  --even `fileprivate` members, because they are bounded by their `private` type
*/
struct Container {
  /*
	VISIBLE IN THIS SCOPE:
	Container
	Container.PrivateStruct1
	Container.PrivateStruct1.VeryFilePrivate1
	Container.PrivateStruct1.VeryFilePrivate2
	Container.PrivateStruct1.x1
	Container.PrivateStruct1.y2
	Container.PrivateStruct1.x3
	Container.PrivateStruct1.y4

	VISIBLE IN THIS SCOPE ONLY IN SWIFT 4:
	Container.PrivateStruct2
	Container.PrivateStruct2.VeryFilePrivate1
	Container.PrivateStruct2.VeryFilePrivate2
	Container.PrivateStruct2.x1
	Container.PrivateStruct2.y2
	Container.PrivateStruct2.x3
	Container.PrivateStruct2.y4

	INVISIBLE IN THIS SCOPE:
	Container.PrivateStruct1.VeryPrivate1
	Container.PrivateStruct1.VeryPrivate2
	Container.PrivateStruct2.VeryPrivate1
	Container.PrivateStruct2.VeryPrivate2
  */
  private struct PrivateStruct1 {
    /*
    VISIBLE IN THIS SCOPE:
    Container
    Container.PrivateStruct1
    Container.PrivateStruct1.VeryPrivate1
    Container.PrivateStruct1.VeryPrivate2
    Container.PrivateStruct1.VeryFilePrivate1
    Container.PrivateStruct1.VeryFilePrivate2
    Container.PrivateStruct1.x1
    Container.PrivateStruct1.y2
    Container.PrivateStruct1.x3
    Container.PrivateStruct1.y4
    
    VISIBLE IN THIS SCOPE ONLY IN SWIFT 4:
    Container.PrivateStruct2
    Container.PrivateStruct2.VeryFilePrivate1
    Container.PrivateStruct2.VeryFilePrivate2
    Container.PrivateStruct2.x1
    Container.PrivateStruct2.y2
    Container.PrivateStruct2.x3
    Container.PrivateStruct2.y4
    
    INVISIBLE IN THIS SCOPE:
    Container.PrivateStruct2.VeryPrivate1
    Container.PrivateStruct2.VeryPrivate2
    */
    private struct VeryPrivate1 {}
    private typealias VeryPrivate2 = Int

    let x1 = VeryPrivate1() // warning(3) error(4)
    let y2 = VeryPrivate2() // warning(3) error(4)

    fileprivate struct VeryFilePrivate1 {}
    fileprivate typealias VeryFilePrivate2 = Int

    let x3 = VeryFilePrivate1() // ok
    let y4 = VeryFilePrivate2() // ok
  }
}

extension Container {
  private struct PrivateStruct2 {
    private struct VeryPrivate1 {}
    private typealias VeryPrivate2 = Int

    let x1 = VeryPrivate1() // error(4)
    let y2 = VeryPrivate2() // warning(3) error(4)

    fileprivate struct VeryFilePrivate1 {}
    fileprivate typealias VeryFilePrivate2 = Int

    let x3 = VeryFilePrivate1() // ok
    let y4 = VeryFilePrivate2() // ok
  }
}

The trouble is, SE-0025 calls for an access control system that essentially nobody can reason about. But it is unambiguous if you sit down and work it out.


[*] And why, you might ask, does SE-0025 call for an access control system where the implicit access control is internal? Because otherwise it would not be possible to make the members of a private nested type visible within the same scope as that of the type, since that access level cannot be spelled within the scope, where private means visible only within the scope of the type. Put another way:

private struct A {
  private struct B {
    private struct C { } // is *more private* than B
    struct BB { }        // implicitly internal, thus *exactly as private* as B
  }
}
1 Like

Disclaimer: I didn’t closely follow SE-0025 back then but access control in Swift 4 is pretty much how I’d like access control to work :speak_no_evil:


FWIW, I find the errors in that code in Swift 4 mode perfectly reasonable. They are exactly what I expected to see after reading the code. If anything, I think the fileprivate declarations inside a private type do exactly nothing and could maybe emit a warning.


My redundant interpretation: x1 and x2 must produce errors because their types are inaccessible outside their defining scope. For example, a method defined on struct Container itself may access members without an explicit access control keyword of PrivateStruct1 (because their scope is the scope of struct Container), but it may not access private members of PrivateStruct1 (because the private scope of PrivateStruct1 is hidden from members of Container).

If x1 and x2 implicitly had the private access control keyword attached to them, that would make them inaccessible from members of Container. If that were the default, I think that would make it harder to actually do something useful with a type like PrivateStruct1. In my experience, when I declare a private nested type, I want to make that type accessible to only the enclosing type. But I still want to access the private type’s members from the enclosing type by default.

Approaching this argument from another side, let’s assume members of a private nested type were themselves implicitly private. How would you declare a property that was usable from the enclosing type then? Using internal like in the following example?

struct Container {
    private struct PrivateStruct {
        let implicitlyPrivate = 1 // private because PrivateStruct is private
        internal let explicitlyInternal = 2 // internal? really?
    }

    func foo() {
        let ps = PrivateStruct()
        ps.implicitlyPrivate // error: 'implicitlyPrivate' is inaccessible due to 'private' protection level
        ps.explicitlyInternal // OK
    }
}

Explicitly having to declare a member on a private type as internal feels strange to me, even if that may be the way it works under the hood right now.

To reiterate: I think the current behavior as observable in the original code is fine.