private vs public *syntax* strategy, as well as other visibility tools like external/api/etc.

[chandlerc]

We should decide how we want to mark public members of record types (structs or classes depending on the resolution of #651) vs. private members.

Lots of folks seem very interested in avoiding the use of a "region" the way C++ works because this can make it hard when jumping to declarations (using whatever tool, but potentially an IDE) to understand the access control.

Note: I'm not suggesting that we have exactly public and private (from C++) members. At least we need some analogy to protected from C++, and we may want something like "module internal" as well.

One suggestion that seems quite popular:

• Make public the default, and have no annotation at all.
• Require a local private annotation on any declaration that should be private
• Similarly for any other non-public case: add a local annotation.

The result as an example here:

class Employee {
  // this would be public
  fn GetName() -> String;

  // this would be private
  private fn SerializeAsJSON() -> String;

  // this would be public
  fn GetID() -> Int;
}

The rationale for this particular set of rules is a bit surprising, so I'll try to walk through them informally here... Much of this is lifted from Discord Chat #syntax channel.

The reason to annotate the private members locally is to reduce the degree of distant and potentially difficult to locate context. @josh11b is proposing a principle in #646 to help codify the reason reducing the contextual load in general, and this seems like a reasonably compelling case to apply this principle.

The reason to make public members be the default is because their readability is a significantly higher priority -- the public API is what we would expect to be cited most and read most of everything. The implementation details (whether private as in C++ or a module-internal or even protected) have a lower readability cost of any annotations. Similarly, data members are often the most common things to be private, and are likely to have a lower readability hit from the annotation than functions as they are a simpler construct in general (no parameters, etc).

Making public the default isn't completely novel either: Kotlin does this as well

We could consider making an optional public specifier (which Kotlin does), but this seems prone to forming needless divergent styles and dialects of code. We couldn't teach people to expect a public specifier because not all code would use it. But especially coming from C++, that might be an easily made mistake.

So the suggestion is to have saying nothing, which we understand always as a very significant impact, to be the way of saying "public".

And following from this, we suggest that a similar approach should be adopted for the exported declarations of a Carbon library. This would move away from marking those explicitly with api and have it be the default. Here, it is important to note that we can reliably check that any declaration in an implementation file either matches an exported declaration in an API file or is correctly marked as internal (or however it ends up being spelled). Again, the goal here isn't to suggest the exact semantic set of markers, but rather the strategy of the public API being the default, and the implementation details carrying tho annotations, as the readability cost of those annotations is lower due to them not being part of the public API.

[chandlerc]

Similar to the switch from an API file to an implementation file allowing reliably detecting a missing annotation, we could have a language feature for a common practice in C++ of a comment in the class (or file) of a comment that says "implementation details only below". This would basically be a syntax that would preclude declaring new public APIs afterward, insisting that anything either match an already declared public API (perhaps to define it) or be annotated in some non-public way. (Credit to @geoffromer in part for suggesting this pattern.)

[zygoloid]

The reason to make public members be the default is because their readability is a significantly higher priority

I find this argument to be compelling. I'm convinced we should annotate access locally, with the absence of an annotation for the public interface and an explicit private (or whatever else) on each other declaration -- even though this will mean that we end up with at least two introductory keywords on every field (private var name: Type;).

[chandlerc]

Confirmed with @KateGregory as well so closing as resolved. (Still needs to be incorporated into a proposal at some point.)

[jonmeow]

Reopening per this discussion on #752 ("api file default public"). We'd like clarification on two scenarios:

• Whether a keyword (private) needs to be repeated between redeclarations, as in:

  class Foo {
    private fn Bar();
  }
  
  fn Foo.Bar() { DoSomething(); }
  

  • Note more generally the decision of whether to repeat could apply to other keywords beyond visibility, such as const, virtual, or override.

• Whether visibility keywords need to be specified in impl files more generally [ed: for file-level entities; they're still applicable for members in classes in impl files]. Keywords here may be redundant (potentially noisy, but also making behavior explicit):

  • For anything forward-declared in the api file, the impl visibility could be determined from the api file.
  • For anything not forward-declared in the api file, an impl file can only add private entities AFAIU.

[chandlerc]

I've been trying to recall this discussion, and I think my memory is somewhat partial... but also the discussion may not have completed. Anyways, writing down what I have in my head so it isn't lost. =D

We didn't discuss the first point around redeclarations of methods that I recall.

We did discuss the impl file case. The concern with omitting the private there was that it would make it difficult to diagnose if, for example, you declare a function in the api file and then have a typo in the definition in the impl file.

But maybe this isn't as bad as imagined? If all the uses of the function use the publicly declared name, the typo'ed definition would be diagnosed as unused. Any uses of the publicly declared name would eventually be diagnosed as not having a definition. And if by some chance every use finds the private declaration, we could still check when we finish building a library that there was some definition for all public declarations.

Another thing to think about would be moving code back and forth between the api file and impl file and having identical code change meaning merely because of where it is written.

Anyways, definitely good to check that we actually got to a coherent consensus across all these cases, and especially good to check the case of redeclaring member functions as that seems important and not really considered.

[zygoloid]

Related to moving code back and forth between the api and impl: we should expect people to copy-paste between a forward declaration and an out-of-line definition (by which I mean, a definition with a prior forward declaration), so we should have easy rules for how to syntactically transform the code when doing so.

I think it's important that an out-of-line definition retains information from the declaration that's necessary to understand that definition. I don't think it's important that it retains information that's necessary to understand the interface, however: readers interested in the interface should be reading the api declaration or (for a class member) the declaration inside the class. As an example, in C++ we see people writing /*static*/ on out-of-line definitions of static member functions but not /*virtual*/, and I think that's because static-ness is important to understand the definition of the member but virtuality is (usually) not.

Suggestion: omit from an out-of-line definition any modifiers that affect only the interface and not the definition (public, private, internal, virtual, abstract, impl, attributes that affect the interface) and retain any modifiers that affect the interpretation of the definition (such as anything about the types of the parameters and me). Given our current syntactic choices, I think that happens to give a simple rule for copying from a forward-declaration to a definition of a function: in the definition, everything before the fn keyword is removed (and Class. is added if necessary).

I think it's OK to omit the private when introducing a new name in the impl case, for the reasons described in @chandlerc's comment -- I think we can reasonably have a whole-library step that checks every name declared in the api section has a definition, so the risk of an api declaration and an impl definition not lining up due to a typo or similar is minimized. There's a consistency loss here, though; the default access changes between the api case and the impl case. I'm not sure whether the consistency is worth more or less than the reduced typing, but I'm very slightly inclined to prefer requiring an explicit private when introducing a new name in an impl.

If we go with this direction, we'll also need to decide whether these modifiers are disallowed in out-of-line definitions, or if they merely become optional. Perhaps disallowing them for now and reconsidering based on feedback would be a reasonable path forward.

[jonmeow]

If we go with this direction, we'll also need to decide whether these modifiers are disallowed in out-of-line definitions, or if they merely become optional.

I believe they should be disallowed; where possible, there should only be one right way of doing things, and people may be misled that removing the extra keywords in a definition has an effect, forgetting to change the declaration.

[jonmeow]

I should add, optional (neither required nor disallowed) redundant keywords in impl files could also confuse readers. Consider:

api:

class Foo {
  private fn Bar();
  private fn Wiz();
};

impl:

fn Foo.Bar() { ...impl... }

private fn Foo.Wiz() { ...impl... }

fn Baz() { ...impl... }

A naive reader of the impl file may conclude that both Bar and Baz are public. After all: the author put in the effort to mark Wiz as private, so clearly the other two are not.

[chandlerc]

Agree with all of @zygoloid 's comment except one tweak:

I'm not sure whether the consistency is worth more or less than the reduced typing, but I'm very slightly inclined to prefer requiring an explicit private when introducing a new name in an impl.

Here I would not. I think the default switching in an impl file makes sense because its not just a default, its the only thing you can say.

And similar to @jonmeow's example, I think it would result in confusion:

api:

private fn Foo();

impl:

private fn Bar() { ... }

// This is still a private function.
fn Foo() { ... }

private fn Baz() { ... }

Forcing users to write private on new functions defined in the impl file seems like it will just create surprise when it then isn't forced for redeclared functions. It's not inconsistent, it's just going to create unnecessary surprise IMO with little readability benefit.

And similarly, requiring private on the definition seems like it'll create a lot of unnecessary ceremony with relatively little benefit to the reader (as outlined above).

The only downside I really see here is that naively moving a definition from an impl file to an api file (where there never was a prior declaration) changes a function from private to public, but that honestly doesn't seem that surprising to me. It's just a minor inconvenience on a relatively rare refactoring. Making the code less noisy and more readable seems more important.

If we go with this direction, we'll also need to decide whether these modifiers are disallowed in out-of-line definitions, or if they merely become optional. Perhaps disallowing them for now and reconsidering based on feedback would be a reasonable path forward.

Agree with @jonmeow that they shouldn't be optional.

[zygoloid]

OK, I'm happy with that tweaked approach. So to summarize:

• The first declaration can contain these keywords, as described in Placement of the word virtual #754.
• In the absence of a keyword, the default visibility is the most public that the declaration can be (which in impl is private), in order to make the public interface readable.
• Subsequent declarations (ie, a separate definition) cannot contain these keywords.

[jonmeow]

I think this is primarily addressed by #752 and other class-specific design work. Any concerns with considering this as proposed and no longer a proposal TODO?

[chandlerc]

Agreed, this seems covered now.