Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/go/types: API adjustments, cleanup #6634

Closed
gordonklaus opened this issue Oct 21, 2013 · 14 comments
Closed

x/tools/go/types: API adjustments, cleanup #6634

gordonklaus opened this issue Oct 21, 2013 · 14 comments

Comments

@gordonklaus
Copy link
Contributor

- export fields from the various Type structs instead of providing accessor methods,
e.g., Map.Key, Pointer.Base
- NewMap should validate its key Type
- expose Convertible, ValidMapKey, others?
- consistent naming of predicates IsIdenticalTo, IsAssignable:  trim both "Is"
and "To"
@griesemer
Copy link
Contributor

Comment 1:

Owner changed to @griesemer.

Status changed to Accepted.

@gordonklaus
Copy link
Contributor Author

Comment 2:

I just noticed that reflect.Type has methods Implements, AssignableTo, and
ConvertibleTo.  Maybe go/types could use the same idiom for these asymmetric methods,
while leaving the symmetric Identical as a func.

@griesemer
Copy link
Contributor

Comment 3:

Merged from 6668:
- what factory functions
- what accessors
- accessors vs direct field access

@griesemer
Copy link
Contributor

Comment 4:

Issue #6668 has been merged into this issue.

@rsc
Copy link
Contributor

rsc commented Nov 27, 2013

Comment 5:

Labels changed: added go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 6:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 7:

Labels changed: added repo-tools.

@griesemer
Copy link
Contributor

Comment 8:

Status update:
- more consistent naming of _exported_ predicates done (dropped leading "Is")
- AssignableTo, ConvertibleTo, Implements exported

@gordonklaus
Copy link
Contributor Author

Comment 9:

One addition:
 - export Comparable
and a suggestion:
 - the "static bool" parameter to Implements is a confusing.  Move the static=false case into its own function named, e.g., Assertible or CanAssert or ValidTypeAssertion
On a related note, the doc for MethodSet.MissingMethod needs to be fixed:  The last bit
"...where x is of interface type typ)"
should read
"...where x is of interface type V)"

@gordonklaus
Copy link
Contributor Author

Comment 10:

I wasn't sure if my previous suggestion made sense or not so I went ahead and
implemented it:
https://golang.org/cl/58540044/
OK if I mail this change?

@gopherbot
Copy link

Comment 11 by MWSherman:

Glad to see Comparable is exported, that’s handy. Any interest in exporting the other
predicates, like isOrdered et al?

@gordonklaus
Copy link
Contributor Author

Comment 12:

IsOrdered et al are available in Basic.Info.  I'm hesitant to add helper functions that
wrap only a type assertion and a bit check; it makes the API busier than necessary.

@gopherbot
Copy link

Comment 13 by matt@stackoverflow.com:

@gordon Legitimate argument, they really are small helpers. I’d be tempted to have a
single helper along the lines of would be handy:
func HasBasicInfo(typ Type, b BasicInfo) bool {
        t, ok := typ.Underlying().(*Basic)
        return ok && t.info&b!= 0
}
(Not a great name but you get the idea.)

@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@rsc rsc changed the title go.tools/go/types: API adjustments, cleanup x/tools/go/types: API adjustments, cleanup Apr 14, 2015
@rsc rsc modified the milestones: Unreleased, Unplanned Apr 14, 2015
@rsc rsc removed the repo-tools label Apr 14, 2015
@griesemer
Copy link
Contributor

Several (most?) of the suggestions have been implemented; it's too late for the remaining ones. The API may be extended but otherwise is now frozen.

@golang golang locked and limited conversation to collaborators Aug 5, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants