reflect: clarify behavior for unexported names #4876

Closed
adonovan opened this Issue Feb 22, 2013 · 6 comments

Comments

Projects
None yet
3 participants
@adonovan
Exported field names inhabit a global namespace but unexported (lower case) field names
belong to the namespace of the package in which they lexically appear.

The "reflect" API doesn't mention this, which can lead to surprising results
when searching for a lowercase field by name.

   func (v Value) FieldByName(name string) Value
   FieldByName returns the struct field with the given name.
   It returns the zero Value if no field was found.
   It panics if v's Kind is not struct.

In the example below, a struct has two fields, both called r, but belonging to different
namespaces.  The "reflect" algorithm, when searching for "r", finds
both, and assumes there's a conflict when in reality there isn't.  Without changing the
existing API, that's the best we can do, but it would be useful to document this because
otherwise the choice of unexported field names inside one package can have subtle
effects on another package using reflection.

http://play.golang.org/p/WTj5d06CQ3

(gri: I know you know all this already.)
@adonovan

This comment has been minimized.

Show comment
Hide comment
@adonovan

adonovan Feb 22, 2013

Comment 1:

(gri -> rsc)

Owner changed to @rsc.

Comment 1:

(gri -> rsc)

Owner changed to @rsc.

@rsc

This comment has been minimized.

Show comment
Hide comment
@rsc

rsc Feb 22, 2013

Contributor

Comment 2:

It's tempting to just remove support for lower case names from FieldByName.
The current behavior is arguably incorrect.
Contributor

rsc commented Feb 22, 2013

Comment 2:

It's tempting to just remove support for lower case names from FieldByName.
The current behavior is arguably incorrect.
@adonovan

This comment has been minimized.

Show comment
Hide comment
@adonovan

adonovan Feb 22, 2013

Comment 3:

Perhaps the most correct solution is to add another method:
 // Precondition: !ast.IsExported(fieldname) <=> pkgpath != ""
 FieldFieldByQualifiedName(pkgpath, fieldname string)
which also compares the StructField.PkgPath, not just the name.
The first method could be changed in one of the following ways:
(a) document its tricky behaviour but remain unchanged.
(b) reject lowercase field names.
(b) accept lowercase field names, and in that case reflect over the callstack (!) and
extract the default package name from the calling frame, and then pass that to the
qualified 2-arg function.

Comment 3:

Perhaps the most correct solution is to add another method:
 // Precondition: !ast.IsExported(fieldname) <=> pkgpath != ""
 FieldFieldByQualifiedName(pkgpath, fieldname string)
which also compares the StructField.PkgPath, not just the name.
The first method could be changed in one of the following ways:
(a) document its tricky behaviour but remain unchanged.
(b) reject lowercase field names.
(b) accept lowercase field names, and in that case reflect over the callstack (!) and
extract the default package name from the calling frame, and then pass that to the
qualified 2-arg function.
@rsc

This comment has been minimized.

Show comment
Hide comment
@rsc

rsc Mar 12, 2013

Contributor

Comment 4:

Leaving for Go 1.2. Not important enough.

Labels changed: added priority-later, go1.2, removed priority-triage, go1.1maybe.

Contributor

rsc commented Mar 12, 2013

Comment 4:

Leaving for Go 1.2. Not important enough.

Labels changed: added priority-later, go1.2, removed priority-triage, go1.1maybe.

@rsc

This comment has been minimized.

Show comment
Hide comment
@rsc

rsc Sep 10, 2013

Contributor

Comment 5:

Labels changed: added documentation.

Contributor

rsc commented Sep 10, 2013

Comment 5:

Labels changed: added documentation.

@rsc

This comment has been minimized.

Show comment
Hide comment
@rsc

rsc Sep 13, 2013

Contributor

Comment 6:

This issue was closed by revision 7fb3d8e.

Status changed to Fixed.

Contributor

rsc commented Sep 13, 2013

Comment 6:

This issue was closed by revision 7fb3d8e.

Status changed to Fixed.

@rsc rsc added this to the Go1.2 milestone Apr 14, 2015

@rsc rsc removed the go1.2 label Apr 14, 2015

@golang golang locked and limited conversation to collaborators Jun 24, 2016

This issue was closed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.