cc @jba

We've talked about having apidiff ignore changes to definitions that are annotated like this. I imagine it would be useful for other tools as well.

What are next steps for getting approval for this proposal?

@carnott-snap I don't think this really needs to go through the formal proposal process. It's something we agree on and discussed already as part of the apidiff and gorelease design.

I think the next step would be for this to be implemented in golang.org/x/exp/apidff. gorelease will use that.

This should be documented somewhere, but I'm not sure where yet. We should have documentation on preparing a release, linked from https://golang.org/doc/. That should basically be a checklist, and gorelease can refer to it in error messages. These stability comments could be mentioned there.

About the specifics on the comments: I agree that // Unstable: is the best single marker for this. I wouldn't mind having // Experimental: as a synonym, but it's probably better to have one word instead of two. We should require the word to be at the beginning of a paragraph, though it doesn't have to be (and usually should not be) the first paragraph.

@carnott-snap I don't think this really needs to go through the formal proposal process. It's something we agree on and discussed already as part of the apidiff and gorelease design.

Is it worth getting this closed/tagged as Proposal-Accepted, or should we start work to prove the benefit first?

I think the next step would be for this to be implemented in golang.org/x/exp/apidff. gorelease will use that.

Does Google want to own the dev work? If not, I am happy to contribute.

This should be documented somewhere, but I'm not sure where yet. We should have documentation on preparing a release, linked from https://golang.org/doc/. That should basically be a checklist, and gorelease can refer to it in error messages. These stability comments could be mentioned there.

Long term it would be nice to mentioning this as part of the module release docs item in #33637. But for now, I think we can follow the lead that // Deprecated: ... has set:

• Create https://github.com/golang/go/wiki/Unstable.
• Amend the godoc blog post: https://blog.golang.org/godoc-documenting-go-code.

About the specifics on the comments: I agree that // Unstable: is the best single marker for this. I wouldn't mind having // Experimental: as a synonym, but it's probably better to have one word instead of two.

Totally agree, may be worth getting a wider audience, but one acceptable word seems better than two perfect ones.

We should require the word to be at the beginning of a paragraph, though it doesn't have to be (and usually should not be) the first paragraph.

IIRC, doc comments rules will not allow it to be the first paragraph, though type unstable struct{} // Unstable: do not use. should be fine. I think we should follow the // Deprecated: ... conventions, and require the keyword be the first word of the paragraph.

Adding "// Unstable:" has a much larger effect than "// Deprecated:".
"// Deprecated" is basically a courtesy: it says "this will keep working but just so you know there is a newer thing you might be happier with." If you don't see the comment, no big deal.

"// Unstable" is much more invasive. It says "even though you might think otherwise, I reserve the right to change the types/code here in the future or delete it entirely and break your uses." If you don't see the comment, it's a very big deal: you upgrade and your code breaks.

The big question is not what the comment syntax should be but whether we want to bless this kind of user-hurting behavior at all.

An alternative would be to develop experimental changes like this on explicitly experimental version tags (v1.6.1-unstable, for example), keeping the unstable code completely out of the stable tag.

Another alternative would be to put the name into the defined symbols, like "mypkg.UnstableFoo", like we did for types like testing.InternalBenchmark (before internal directories). It's impossible to miss the Unstable when it's in the name.

We should very carefully consider what the right way is to let package authors experiment without hurting users. A simple comment does not seem like enough. (I realize that the idea is tools would surface the comment etc but then that's just more mechanism on top, whereas versions or symbol names that explicitly say unstable reuse existing mechanism.)

The big question is not what the comment syntax should be but whether we want to bless this kind of user-hurting behavior at all.

Note that there are already at least a few examples of unstable definitions in the standard library.

For example, consider:

• unicode.Version (which changes every time the version is updated)
• os.ModeType (which would change if a new type bit is added)
• text/scanner.GoTokens (which will change if a new kind of token is added to the Go grammar, for example in order to handle generics).

As another data point, there are also several symbols in the testing package: RegisterCover, Cover, CoverBlock, MainStart. These have comments like:

It is not meant to be called directly and is not subject to the Go 1 compatibility document. It may change signature from release to release.

These definitions are only meant to be called by generated code that is tightly coupled with the testing package, so in this case, it might be fine to name things Unstable. I imagine you might see the same pattern any time you have a code generator and a library that go together.

However, some definitions like the ones Bryan mentioned may have their own definition of compatibility. According to apidiff, changing the value of an integer constant is an incompatible change because it may be used as an array length. Using unicode.Version as an array length would be quite strange though, and I don't think there's a need to report a change to that constant as an error in gorelease. I'd like to have some annotation like // Unstable: that gives authors a chance to suppress false positives like this.

@rsc I'm not sure what the next step is. It doesn't sound like the proposal committee has accepted or rejected this. What information would be useful in making a decision?

whether we want to bless this kind of user-hurting behavior at all.

As a data point. In the protobuf module, there are several types or functions explicitly marked as being for internal use only. However, they must be exported in order for generated code to access them. We reserve the right to change those APIs with the caveat that we don't break existing usages that were properly generated by protoc-gen-go.

Whether this convention is adopted or not, this type of sharp edge already exists. In v1 the sharp edge is awful since the internal functions is placed alongside public functions in the proto package. In v2, the sharp edge is isolated to a protoimpl package which is explicitly documented as being unstable.

@jayconrod I think that what might be helpful is a reason why you can't use names like ExperimentalFoo.

@ianlancetaylor My expectation is that gorelease via apidiff will identify a number of changes as technically incompatible, but developers will want a way to annotate definitions to suppress false positives.

I'm planning to ship an experimental version of gorelease soon ("soon" depends on what other cmd/go issues need to be fixed before the 1.14 beta). Hopefully I'll have some useful feedback to share after that.

Sorry if this is an unhelpful comment, but the cmd/api tool in the standard library does have a way to suppress false positives, for the kinds of examples that @bcmills cites above. See the files in https://golang.org/api/ .

I think that what might be helpful is a reason why you can't use names like ExperimentalFoo.

• In the description I call out why Experimental is problematic for long term, intentional instability: is Unstable equally acceptable?
  • If so, s/Experimental/Unstable/g.
• Experimental is quite long to add to every unstable symbol.
• Would const experimentalFoo = "bar" be stable?
• Would this apply to functions retroactively?
  • type ExperimentalRoute interface{} where the link hardware is experimental, not the Go api.
  • github.com/moby/moby@v1.13.1/api/server/router.ExperimentalRoute is stable
• What is the migration path for symbols that are partially stable? (read: custom compatibility guarantees)
  • It would be a breaking change to convert gRPC code generator's logic fromtype XxxService interface into type ExperimentalXxxService.
• Must Experimental be the first word, or simply prefix in the symbol?
  • func ExperimentallyInvestigate() {}
  • func GetExperimentalFoo() {}
• How do I mark a whole package or module?
  • Can we reserve exp or experimental, like we did internal?
  • Would a lowercase prefix be clear?
    • syscall/experimentaljs
    • golang.org/x/exp/experimentalapidiff
  • Is documentation sufficient?
    • See the current state of syscall/js, golang.org/x/exp, etc.
  • Currently apidiff and gorelease are not mindful of this.
  • golang.org/x/exp/apidiff.ExperimentalChange has stutter.
  • What protects you from accidentally committing to something: golang.org/x/exp/apidiff.Change.

I didn't mean anything special about the exact word Experimental. I just meant, as in #34409 (comment), to put the instability into the symbol name itself rather than a comment. A comment can be missed. It's much harder to miss the symbol name.

The (IMO pretty major) downside of putting words like Experimental in the identifiers (or in the package path, for that matter) is that it imposes an additional breaking change if/when the API is promoted from experimental to stable.

(It's the same reason we don't distinguish between v0 and v1 in module paths.)

It's much better to use branches for experimental features. This makes 2 worlds(stable and experimental) independent and reduces possible mistakes for the user.

As Russ mentioned above: comment is too weak to protect user from incorrect usage.

Extending previous comment by Bryan: v0 and v1 means that the module can be experimental, but in the same time we can use replace statement in go.mod to use a module from specific branch with experimental/unstable/ things.

In other words: comments or unstable api increases code entropy which increases code maintainability.

I built this proposal around documentation to make adoption less invasive. This was partially based on experience with the protobuf libraries, where they define custom compatibility guarantees. I agree condoning unstable interfaces is troublesome, but many projects require it, and I wanted a canonical way to label/identify it, both for users and tooling.

It may be helpful to isolate the use cases that exist for instability: (please correct me if I left anything out or if this is a false trichotomy)

1. We have experimental things and want to change/remove them.
   • Call this the pre-release case (or alpha, beta).
   • packages
     • golang.org/x/exp
     • syscall/js
   • symbols
     • golang.org/x/tools/go/analysis.ObjectFact
2. We would like to define our own compatibility guarantees that differ from the apidiff/Go 1 compatibility.
   • Call this the we know better than the language case.
   • unicode.Version
     • This will not randomly become a func, but the value and length may change.
   • Call site compatibility
     • Protobuf generated code type XxxServer interface must be implementable and allow adding methods.
   • Considering the complexity of the apidiff spec, this will keep cropping up; sometimes it could be useful to break some guarantees to give extra features: e.g. change a constant's value when you know it will not be used as an array length.
3. We want to share an internal interface between two packages.
   • Call this the protected api surface case.
   • testing.RegisterCover
   • gRPC generated methods against message structs: e.g. XXX_Unmarshal.
   • Some of these are required because of the limited nature of the Go language, others are bad practice, frequently it is hard to tell the difference.

I think we all agree that we want stable packages, but even the standard library is developing experimental or long term unstable features. Most examples use documentation to signal their stability, so this seemed intuitive and canonical. What emphasis should be placed on supporting existing patterns, as opposed to preventing undesirable behaviour?

Outstanding concerns:

• Branches and tags will not work well with 2. or 3. Authors want the ability to ship official, (partially) unstable releases.
• The Experimental proposal does not appear to be a specification, so much as a convention. This lack of tooling support makes it incompatible with my intentions.

@rsc I'm not sure what the next step is. It doesn't sound like the proposal committee has accepted or rejected this. What information would be useful in making a decision?

The proposal committee's job is to help steer a discussion toward consensus, not to decide on their own. It sounds like there is no consensus here yet.

I wrote a lot about this topic at https://research.swtch.com/proposals-experiment. It's very important that people know they are using experimental features. Otherwise we run the risk of hitting the situation Rust did where everyone was on "unstable". I have the same concern about the "exp" build tag in #35704: it's too easy to lapse into where everything uses this.

Actually the build tag may be worse, since if any dependency super far down uses exp, you have to do the same. So basically everyone will have to use it.

I would like to be clear about the purposes of the two tickets being discussed:

• proposal: cmd/doc: add "// Unstable:" prefix convention #34409: documenting any form of feature instability
• proposal: doc: add "exp" build tag convention #35704: gating alpha, beta, testing, pre-release features that will be stabilised

The proposal committee's job is to help steer a discussion toward consensus, not to decide on their own. It sounds like there is no consensus here yet.

My concern is that the current state of things is not good. Major tools are actively developing and releasing unstable features, by apidiff's measure, that have a myriad of ways to signal their stability guarantees. This is hard on consumers, and may lead to more instability if we do nothing.

I wrote a lot about this topic at https://research.swtch.com/proposals-experiment. It's very important that people know they are using experimental features.

Would a go vet lint be sufficient? We already do this for // Deprecated: . What do you define as know they are using? This seems like a very tricky litmus test, e.g. dep was relied upon heavily by the community, despite never leaving experiment status.

Otherwise we run the risk of hitting the situation Rust did where everyone was on "unstable".

I have worked with Rust myself and do not see this today. Are there any lessons we can learn from their experiences? My understanding was that they had a lot of important features that needed to be stabilised, e.g. async, and that finally unlocking things from the nightly compiler was the fix.

I have the same concern about the "exp" build tag in #35704: it's too easy to lapse into where everything uses this.

Actually the build tag may be worse, since if any dependency super far down uses exp, you have to do the same. So basically everyone will have to use it.

Since #35704 is trying to solve a different problem, would you mind continuing the discussion there? I broke it out partially because I saw two heterogeneous problems (pre-release and custom compatibility) that felt like they may should be solved differently.

Maybe this has already been suggested somewhere, but would it not be better to support all tags of the form // TagName:, and then allow filtering for including or excluding the items tagged with TagName, and by default, group tagged items together? Like that we can avoid this discussion on whether or not a specific tag is a good idea or not, and also avoid officially "blessing" any tag in particular.

@beoran, I think the point of this proposal is the semantics, not the syntax: if we want to know how to identify an unstable API we must “bless” a specific tag as having that meaning, or else no one will know to add it.

Are you suggesting extending that proposal to also work for a given module version?

@codyoss I think Russ means the same version. Module A declares go 1.16 in its go.mod; that could apply restrictions to "marked for removal" APIs its packages use, from either the standard library or other modules.

I agree in seeing #30639 succeed first; it seems like a good idea on its own anyway, even if it's just for being able to clean up the standard library API or v1 modules that don't necessarily need a v2.

If we then find that the other proposal is not enough for this use case, I agree with @ianthehat and @bcmills that build tags seem like the best approach for marking code as unstable/experimental/etc.

If an API is marked unstable it does not necessarily mean that it will be removed or changed in the future. I would hope that an API, even experimental one, would have a lot a time and thought put into it. I would say the goal is to promote the API to a stable-state and if possible do so without changing the signature at all. Sometimes you can't be sure if a signature is right though without more feedback from the consumers of the API.

@codyoss, that still seems like an appropriate role for build constraints..?

You can start with the API guarded by some tag (perhaps unstable if you want to use a general tag, or grpc_naming or similar if you want a per-feature tag), and then remove the build constraint once the API is considered “stable”.

@bcmills

that still seems like an appropriate role for build constraints..?

Yes, I agree. I should have been more clear in my response. I was more so responding to proposal being linked and the fact that it solves the "what if I want to remove it" and not the "I want to promote it" issue. For this reason I don't think that the proposal, #30639, should necessarily be a blocker for this issue.

Build constraints don't work well when you have an API that you wish to declare unstable externally but use internally to implement stable features.

Now that we have type aliases, this may be possible now, but type aliases still leave godoc as a total mess.

@dfawley, you can certainly use internal packages to implement stable features.

If you want, you can even apply build tags such that the internal package uses type aliases to wrap the unstable public package (not the other way around) when the public package exists, and defines those types itself otherwise. That allows the public package to work around any documentation deficiencies while still making the API available for unrestricted internal use.

That said, it usually should not be necessary: you can usually define the types in the unstable public package as their own struct wrapper types rather than aliases. (That also prevents new exported methods added to the internal types from immediately leaking into the public API.)

Now that we have type aliases, this may be possible now, but type aliases still leave godoc as a total mess.

That seems like one or more issues that should be filed separately.

@dfawley: go doc, godoc.org, and pkg.dev.go all clearly display and link against aliases, e.g. x/net/context.Cancelled, are you looking for them to inline the alias for better readability? #6600 seems related the aforementioned struct example, but I could not find an issue for aliases specifically.

are you looking for them to inline the alias for better readability

Yes, especially in this scenario. Consider a package with an extensive experimental API. Users would have to navigate back and forth between two (or more) packages: one containing the symbols they are allowed to access, and another one (or more) containing the usage documentation, struct fields, interface methods, etc. And where one experimental function accepts an experimental struct as a parameter:

package internal

type MyInterface interface {
  MyFunc(MyStruct) error
}

type MyStruct struct {
  ...
}

package experimental

type MyInterface = internal.MyInterface
type MyStruct = internal.MyStruct

When reading the documentation for internal.MyInterface.MyFunc, you see MyStruct. There is no back-reference to experimental.MyStruct. So you just have to hope the API designers are sane and didn't instead do:

package experimental
type FooBar = internal.MyStruct

In this contrived example, that is obviously bad, but it's possible something like this could end up happening in practice with good intentions behind it.

The first workaround @bcmills mentions above works, but I really don't want to maintain two identical copies of an API. You could generate the internal one by:

//go:generate cp ../experimental_*.go .; sed -i '1s;^;// +build !unstable;' experimental_*.go

...but this is pretty gross. And you have to manually maintain the // +build unstable variant that has a type alias for all the experimental package's symbols.

Moveover, what library developer is realistically going to think to do all of this? The process for creating an experimental API should be simple, straightforward, and a pattern that is obvious or discoverable when the desire to create one arises. Not something that took several engineers debating for months to eventually concoct its final form. Almost nobody will do this. Instead they'll just add a comment, like we do in grpc-go today.

That totally makes sense, and is something I have considered asking for before, probably best to make a new ticket for it, since it would be useful orthogonal to this issue. I will say that rust seems to have solved the alias problem in their docs, inlining them in a way that many times I do not know they are even aliases, but they also give vastly more control over what is and is not included.

what library developer is realistically going to think to do all of this? The process for creating an experimental API should be simple, straightforward, and a pattern that is obvious or discoverable when the desire to create one arises.

The general process for creating an experimental API is:

1. Define the experimental API in a pre-release version.
2. Get users to try out the experimental API using the pre-release and submit feedback.
3. Prior to the next stable release, either declare the API stable or (temporarily or permanently) remove it.

Or:

1. Define the experimental API in a public package guarded with a build constraint.
2. Don't use the experimental API within the implementation of a non-experimental API.
3. When the experimental API becomes stable, remove the build constraint.

Or:

1. Define the experimental API within an internal package.
2. Use the experimental API only internally.
3. When the experimental API becomes stable, move it to a public package.

The extra work of defining forwarding wrappers only comes up as a concern for experimental APIs that simultaneously:

• are expected to remain “experimental” for longer than a release cycle, and
• must be available for public use while still experimental, and
• must be used in the implementation of a non-experimental API while still experimental.

So, yeah: if you're doing something unusual (developing an experimental API, over the course of multiple releases, with public and internal-stable users), then you may need to do some extra work (such as maintaining forwarding declarations) in order to facilitate that. But that is (or ought to be) a rare case, not the default workflow for new APIs.

The general process for creating an experimental API is:

Is this a suggestion or cannon? Can we get this written up in a faq, blog, or wiki page? I am unclear how discoverable this (open) issue is.

1. Define the experimental API in a public package guarded with a build constraint.

What are best practices for build constraints? exp, path_to_my_package_my_special_feature, etc.

The extra work of defining forwarding wrappers only comes up as a concern for experimental APIs that simultaneously:

Are you suggesting that the experimental name in the package path is a convention that we can detect in tooling like apidiff? Not sure exactly what you are suggesting:

• package experimental // import "path/to/experimental"
• package pkg // import "path/to/experimental/pkg"
• package experimental // import "path/to/pkg"
• package experimentalpkg // import "path/to/experimentalpkg"

Not saying they are right, or right for Go, but other languages(libraries) do support the idea of unstable APIs via annotation/comment. Specifically I am thinking of Guava's Beta annotation.

@carnott-snap

The general process for creating an experimental API is:

Is this a suggestion or [canon]?

Neither? It is a description of approaches that can be followed today by combining mechanisms that are each already fairly well-established in Go.

What are best practices for build constraints? exp, path_to_my_package_my_special_feature, etc.

Perhaps that's the thing to decide here? IMO you can't go wrong with a per-feature build constraint, but a catchall unstable or experimental tag might be ok too.

Are you suggesting that the experimental name in the package path is a convention that we can detect in tooling like apidiff?

I am not suggesting the experimental name in the package path at all. (That was @neild's suggestion, and I don't agree with it.)

I am suggesting one of:

• Make the package internal. When it is ready for external use, either move it or add a forwarding package.

  package pkg  // import "path/to/internal/pkg"`
  
  // NewStuff does experimental stuff.
  func NewStuff() { … }

• Make the package externally available from the start, but guard all of its declarations with explicit build constraints.

   //go:build experimental
  
   package pkg  // import "path/to/pkg"
  
   // NewStuff does experimental stuff.
   func NewStuff() { … }

• Both of the above. Make an internal package that always exists, and an external, build-constrained package that forwards to it.

  package pkg  // import "path/to/internal/pkg"`
  
  // NewStuff does experimental stuff.
  func NewStuff() { … }

  //go:build experimental
  
  package pkg  // import "path/to/pkg"`
  
  import internal "path/to/internal/pkg"
  
  // NewStuff does experimental stuff.
  func NewStuff() { internal.NewStuff() }

  If the forwarding wrappers are too tedious to maintain by hand, perhaps use or implement a tool to generate them automatically. (My goforward prototype is a start in that direction, but it's a bit too buggy to use today, and probably biases too far toward using type aliases rather than creating additional struct types.)

Not saying they are right, or right for Go, but other languages(libraries) do support the idea of unstable APIs via annotation/comment. Specifically I am thinking of Guava's Beta annotation.

First, annotations are not comments, since they are available at runtime. IIUC, there is nothing congruent in go.

While I agree that docs are more convenient, the discussion here seems to conclude that they are not enough. I would advocate for people ALSO putting details about the expected duration of experimental symbols in godoc, especially for prereleases intended for testing.

That being said, you highlight a good point: once we define a convention here, godoc/gddo/pkgsite all need to be able to display or warn users about experimental symbols.

Neither? It is a description of approaches that can be followed today by combining mechanisms that are each already fairly well-established in Go.

Considering that so many packages ignore or go their own way for experimental features, I think that a documented canonical solution is necessary, even if you can derive these principles from existing documentation.

Perhaps that's the thing to decide here? IMO you can't go wrong with a per-feature build constraint, but a catchall unstable or experimental tag might be ok too.

Agreed. I think there will always be a place for custom build tags, but having a convention will make the interface easier to use.

I strongly prefer exp to experimental as it is both shorter and nearly impossible to misspell. That being said, several canonical tags may be the better approach, since some symbols are unstable not exp and there are people who seem to like alpha, beta, rc.

I am not suggesting the experimental name in the package path at all. (That was @neild's suggestion, and I don't agree with it.)

Sorry for misunderstanding. I am happy to exclude this from the proposal, unless someone passionate can advocate for it.

If the forwarding wrappers are too tedious to maintain by hand, perhaps use or implement a tool to generate them automatically. (My goforward prototype is a start in that direction, but it's a bit too buggy to use today, and probably biases too far toward using type aliases rather than creating additional struct types.)

Yeah, I have a few major version bumps that I have done with goforward, and I both like it and totally see the benefit, but it is showing its age. Probably good to fork off into a different proposal.

IMO you can't go wrong with a per-feature build constraint, but a catchall unstable or experimental tag might be ok too.

I think a single build tag is OK provided there is tooling support. There needs to be a way to prevent accidentally including unstable code, which would otherwise happen all the time:

1. You take a new dependency. Your program stops compiling because something in the new package that you want is marked unstable.
2. You evaluate the unstable API and decide that you're willing to depend on it. You add -tags unstable to your build script. Everything works. The change to your build script shows up in you PR/CL diff, so reviewers can evaluate the decision to depend on the unstable API.
3. You take another new dependency, which uses some other unstable API (perhaps transitively). But you don't notice because your build script already allows all unstable code, and your reviewers don't notice because there is no diff.

I think a single build tag is OK provided there is tooling support. There needs to be a way to prevent accidentally including unstable code, which would otherwise happen all the time:

Can you be more concrete on what you would like wrt "tooling support". I agree it would be very helpful, but this could be a warning during go build or go get, simply a call out in the godoc/gddo/pkgsite, or something else.

3. You take another new dependency, which uses some other unstable API (perhaps transitively). But you don't notice because your build script already allows all unstable code, and your reviewers don't notice because there is no diff.

I would make the argument that using the unstable tag is you opting into the unstable ecosystem and that you are implicitly accepting more than just the one symbol from the original pr. Otherwise you should really use a feature specific build tag. (Aside, I am a fan of this ecosystem approach, as it allows cross repo experimentation and seems to work well in the rust ecosystem, however others have called out that same ecosystem as a fundamental problem.)

This speaks to your tooling comment, but displaying a warning in gorelease for EVERY symbol gated by unstable (or any build tag) could be sufficient, though we may want to merge it as go release before calling it standard or stable tooling.

A tool that wrote every unstable symbol (or even just package) to stdout would be sufficient. Then the output could be dumped to a file included in the PR, and changes would show up as diffs.

That is how I use gorelease today, but with breaking changes, not experimental symbols. I also want this information visible via godoc/gddo/pkgsite, so maybe exposure via go doc would be better.

Marking on hold until #30639 is decided, per #34409 (comment).

Placed on hold.
— rsc for the proposal review group