spec: embedding a type alias is confusing

When you embed a type alias within a struct, the resulting struct type has a field of the alias's apparent name, but its run-time type descriptor uses the alias's original name. For example:

package main

import "fmt"
import "bytes"

type buffer => bytes.Buffer

type S struct {
        buffer
}

func main() {
        var s S
        _ = s.buffer
        // _ = s.Buffer // compile error: s has no field Buffer
        fmt.Printf("%#v\n", s) // "main.S{Buffer:bytes.Buffer{...}}"
}

This is potentially confusing to the reader, and I suspect it will break a number of analysis tools that assume the name in the syntax of the embedded field declaration is the same as the name of the field.

Something to think about.

It's worse. If the package that exports S is imported again, in that package the anonymous field is named as the type: Buffer rather than buffer. That is, the same field is named differently in different packages.

package t1

import "go/build"

type T struct {
    context
}

type context => build.Context

var t T
var _ = t.context // lower-case context

package t2

import "./t1"

var t t1.T
var _ = t.Context // upper-case context

After clarification in spec, the compiler needs to be fixed.

Since this affects type aliases, this issue would persist even if we restricted aliases to types only.

It should be fairly simple to fix in the compiler: Currently, when we export anonymous fields, we deduce the field name from the type. We simply have to export the actual field name as well (perhaps only when it's different from the type name).

By fix, do you mean treat struct { buffer } as if it had been written struct { bytes.Buffer }?

No, treat struct { buffer } as it's written; i.e. the field name is buffer in this case.

It looks like the public APIs for reflect.StructField and go/types.Var already independently report name and anonymity/embeddedness, so that's good.

However, reflect internally represents anonymous fields as an empty field name, which at runtime it replaces with the field type's name. We'll need to change that to allow named embedded fields.

(Unless we do decide instead that struct { buffer } is equivalent to struct { bytes.Buffer }.)

Using the field name as written seems much clearer. It does mean that some types differing only by alias names are non-identical, such as these three:

struct { bytes.Buffer }
struct { buffer }
struct { buffer2 } // another alias of bytes.Buffer

CL https://golang.org/cl/32633 mentions this issue.

CL https://golang.org/cl/32674 mentions this issue.

@robpike

I believe the behavior that Alan describes in the initial report is the right one. As I said on the CL, think of alias like a #define. Whether you name the alias or the actual definition of the alias, the resulting program should have identical behavior 100% of the time. Because "buffer" is an alias for "bytes.Buffer", embedding "buffer" behaves exactly like "bytes.Buffer".
Analysis tools should probably be working on the "original" type all the time. As long as they "dereference" the alias aggressively, they should have no problem with this. People may have a slight problem, but tooling will help and of course convention will too.

I agree that the embedded buffer being named Buffer is a little bit of a surprise, but the definition of alias is crystal clear about the effect:

The effect of referring to a constant, type, variable, or function by an alias
is indistinguishable from referring to it by its original name.

I think it would be a serious mistake to make exceptions to that rule. Doing so would bar (or at least complicate) automated fixes that try to update code to use non-deprecated names in place of deprecated aliases.

That said, the legacy aliases byte/uint8 and int32/rune do not behave this way: they take the name used in the struct. That implies to me that we should not refer to them as aliases, now that alias has a precise and different meaning. I filed #17766 for a reflect bug involving those.

/cc @robpike @ianlancetaylor

I appreciate that the spec change in https://go-review.googlesource.com/30601 breaks the "crystal clear" invariant you quoted, and makes structs nonidentical if their fields differ only by an alias name, but I think using the name as it appears for the field name leads to much more readable code.

Also, the change you propose would break an equally important invariant: identifier case determines exportedness.

You would be surprised to read this declaration

type S struct { foo }

or perhaps see this documentation in godoc:

type S struct { /* unexported fields suppressed */ }

and discover that struct type S has a field called Foo (or perhaps even Bar).

As for the "automated fixes", the lesson is: when you use aliases as forwarding pointers during a refactoring, be sure to give the alias the same name as the original.

type b => bytes.Buffer
var B1 b
var B2 struct { b }
var B3 struct {
    struct { b }
    struct { bytes.Buffer }
}

I gather that in this example the idea is that B2.b does not work because there is really no field named b, likewise B3.b does not work, and B3.Buffer does not work because there is a name collision.

But what about exports? In this example, does the package have an exported type Buffer? Or given type B => bytes.Buffer does the package have an exported type B or an exported type Buffer?

What is the key difference between those cases? In one case, the aliased name matters, and in the other case the alias name matters?

But what about exports? In this example, does the package have an exported type Buffer?

The type alias b is not exported because it starts with a lowercase letter. The exported type of B2 is a struct containing a single "anonymous" field named "b" of type bytes.Buffer.

and B3.Buffer does not work because there is a name collision.

No, you can't compile even the declaration of B3 because an anonymous field must be a named type or a pointer to one, not an unnamed struct type.

given type B => bytes.Buffer does the package have an exported type B or an exported type Buffer?

The package would have an exported type alias B whose original is named bytes.Buffer and has the declaration struct {...etc...}.

I think not allowing renaming alias makes aliases much less useful. Think rsc's example of io.ByteBuffer and bytes.Buffer. Choosing good package names to simplify exported names is so essential to idiomatic Go style that I expect non-trivial amount of aliases be renaming. Let's not forgetting the non-refactor use of aliases too (integration of multiple internal pacakages into a API only top-level package, renaming aliases would play a key role there. Also, because I expect relaxing this restriction later will likely receive similar resistance like the introducation of aliases received today (because it makes alias abuses much worse, which is the main argument against alias), I think if we make this restriction today, we will never be able to remove it.

I don't understand what the type reflection information looks like for option 3. Are we going to set Name to the alias, set Type to the imported type, and set Anonymous to true? Note that that will require a change to the format of the reflection information currently being generated by the compiler, as it currently has no way to both provide a name and indicate that the type is embedded.

@jimmyfrasche

@jimmyfrasche, thank you for the observation that (1) constrains people writing alias statements and (2) constrains people using code that happens to use alias statements. That's an important distinction, and I think it's a good argument that (1) is preferable to both (1.5) and (2).

@ianlancetaylor

@ianlancetaylor I haven't looked at the reflection information yet. I have a pending CL for the necessary export data change (still missing tests besides my manual tests). It does contain an extra field name if the "name" of the anonymous field doesn't match the base type's name. I suspect something similar will need to be done for reflection info.

@rsc

Rereading the thread, it looks like we're down to option 1 or option 3. It also seems the arguments for and against each hinge on what a type alias means.

Given a type alias in a package p, type T => q.S, everyone agrees that values of p.T and q.S should be able to be used interchangeably regardless of the name used.

The disagreement seems to be whether the name p.T is a "#define" (@rsc) or a "hard link" (@quentinmit).

In the #define interpretation it is as if instances of p.T are replaced by q.S in the text before compilation. They may be used interchangeably in the source but, in a sense, only the original name, q.S, is ever actually used.

The hard link interpretation is that p.T and q.S are two alternate names of the same named type. They are like two distinct but synonymous words.

Option 1 allows both interpretations to behave the same for embedding.

Option 3 uses the hard links interpretation.

Consider

package r
import "p"
type U struct {
    p.T
}

By the #define interpretation, the field name of the anonymous field is S. (This was option 4).

By the hard links interpretation, the field name of the anonymous field is T.

The #define interpretation has the same effect as options 1.5/1.75/2: it constrains people using code that happens to use the alias statement. They have to remember that even though they embedded p.T the field name is S.

One of the major use-cases for aliases is refactoring. Renaming aliases is an important part of that, as mentioned.

Under the #define interpretation, refactoring is inhibited when embedding is used.

Say the type U, defined above, is written while p.T is an alias of q.S and in the code it's accessed by its field name, S. After refactoring p.T is no longer an alias. Suddenly all code accessing S from U be fail to compile and have to be changed to T.

However, under the hard links interpretation nothing has to be changed: the field name is T regardless.

Even if this decision is delayed for now by going with option 1, I think the outcome will still have to be option 3 in the end.

The restriction that aliases can only reference names in a different package already restricts the renaming operations that can be performed. Under the original proposal you can rename p1.T1 => p2.T2, but not p1.T1 => p1.T2.

Extending this restriction to cross-package renames (option 1) doesn't seem onerous to me. If it is, then perhaps that's an argument against limiting aliases to names in different packages.

@neild

@neild You can still rename p.T=>p.U, it just requires extra steps and an extra temporary package during the transition. For example:

Add package q with q.A=>p.T.
Refactor all uses of p.T to use q.A.
Atomically rename p.T to p.U, and change q.A's alias to q.A=>p.U.
Refactor all uses of q.A to use p.U.
Remove package q.

I'm in favour of option 3. The types are indistinguishable in all but name, but naming is precisely what aliasing is all about. It also fits with the existing embedding semantics of built in types such as byte and rune - to define these exactly as aliases would be a nice bonus (issue #17766 is indicative of the confusion in this area).

@neild

@neild

Extending this restriction to cross-package renames (option 1) doesn't seem onerous to me. If it is, then perhaps that's an argument against limiting aliases to names in different packages.

I would argue that it's the latter.

Option 3 seems best from a user perspective: it doesn't break things nearly as badly when moving an existing type out of a package, because you can drop in a forwarding alias and everything still basically works as expected.

FWIW (which isn't much :), I'm strongly in favor of an approach which allows byte and rune to be defined as aliases. It's just simpler.

"An alias can never be lower case" seems to solve the problem and is consistent with the fact that aliasing is about forwarding the exposed object definitions of a package.
Along with the fact that an unexported object should not be aliasable.

Renaming feels quite important in order to avoid naming conflicts and as a way to create parametrized packages (aliases as a kind of naming indirection allows to switch type definitions quite more easily).

Options 4 still makes the most sense to me (aliases should be almost (*) completely transparent at runtime since it is a compile-time mechanism and aliasing has no incidence on type identity, by definition).

Concerning error messages, I guess that the line numbers will help disentangle any discrepancy between alias vs. original name.

( * ) Ideally, there should be local package information when an object is aliased and part of an error message.

Locking because aliases are no longer proposed.

Navigation Menu

Repository navigation

spec: embedding a type alias is confusing #17746

27 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

spec: embedding a type alias is confusing #17746

Description

Activity

griesemer commented on Nov 2, 2016

griesemer commented on Nov 2, 2016

griesemer commented on Nov 2, 2016

alandonovan commented on Nov 2, 2016

griesemer commented on Nov 2, 2016

mdempsky commented on Nov 2, 2016

alandonovan commented on Nov 2, 2016

gopherbot commented on Nov 2, 2016

gopherbot commented on Nov 3, 2016

rsc commented on Nov 3, 2016

alandonovan commented on Nov 3, 2016

ianlancetaylor commented on Nov 3, 2016

alandonovan commented on Nov 3, 2016

27 remaining items

minux commented on Nov 4, 2016

ianlancetaylor commented on Nov 4, 2016

rsc commented on Nov 4, 2016

griesemer commented on Nov 4, 2016

jimmyfrasche commented on Nov 4, 2016

neild commented on Nov 4, 2016

mdempsky commented on Nov 4, 2016

rogpeppe commented on Nov 7, 2016

bcmills commented on Nov 8, 2016

neild commented on Nov 8, 2016

atdiar commented on Nov 8, 2016

rsc commented on Nov 27, 2016

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions