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

Jump to bottom

api: audit for Go 1.17 #46688

Closed
83 tasks done
heschi opened this issue Jun 10, 2021 · 13 comments
Closed
83 tasks done

api: audit for Go 1.17 #46688

heschi opened this issue Jun 10, 2021 · 13 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.17

Comments

@heschi
Copy link
Contributor

heschi commented Jun 10, 2021 •
edited by dmitshur

This is a tracking issue for doing an audit of API additions for Go 1.17 as of https://golang.org/cl/326410.

New API changes for Go 1.17

archive/zip

https://golang.org/cl/312310

  • pkg archive/zip, method (*File) OpenRaw() (io.Reader, error)
  • pkg archive/zip, method (*Writer) Copy(*File) error
  • pkg archive/zip, method (*Writer) CreateRaw(*FileHeader) (io.Writer, error)

OK: accepted in proposal #34974

compress/lzw

https://golang.org/cl/273667

  • pkg compress/lzw, method (*Reader) Close() error
  • pkg compress/lzw, method (*Reader) Read([ ]uint8) (int, error)
  • pkg compress/lzw, method (*Reader) Reset(io.Reader, Order, int)
  • pkg compress/lzw, method (*Writer) Close() error
  • pkg compress/lzw, method (*Writer) Reset(io.Writer, Order, int)
  • pkg compress/lzw, method (*Writer) Write([ ]uint8) (int, error)
  • pkg compress/lzw, type Reader struct
  • pkg compress/lzw, type Writer struct

OK: accepted in proposal #26535

crypto/tls

https://golang.org/cl/295370

  • pkg crypto/tls, method (*CertificateRequestInfo) Context() context.Context
  • pkg crypto/tls, method (*ClientHelloInfo) Context() context.Context
  • pkg crypto/tls, method (*Conn) HandshakeContext(context.Context) error

OK: accepted in proposal #32406

database/sql

https://golang.org/cl/311572

  • pkg database/sql, method (*NullByte) Scan(interface{}) error
  • pkg database/sql, method (*NullInt16) Scan(interface{}) error
  • pkg database/sql, method (NullByte) Value() (driver.Value, error)
  • pkg database/sql, method (NullInt16) Value() (driver.Value, error)
  • pkg database/sql, type NullByte struct
  • pkg database/sql, type NullByte struct, Byte uint8
  • pkg database/sql, type NullByte struct, Valid bool
  • pkg database/sql, type NullInt16 struct
  • pkg database/sql, type NullInt16 struct, Int16 int16
  • pkg database/sql, type NullInt16 struct, Valid bool

OK: accepted in proposal #40082

debug/elf

https://golang.org/cl/239217

  • pkg debug/elf, const SHT_MIPS_ABIFLAGS = 1879048234
  • pkg debug/elf, const SHT_MIPS_ABIFLAGS SectionType

Did not go through proposal process, but new entry in existing type, so OK. - @rsc

encoding/csv

https://golang.org/cl/291290

  • pkg encoding/csv, method (*Reader) FieldPos(int) (int, int)

OK: accepted in proposal #44221

go/build

https://golang.org/cl/310732

  • pkg go/build, type Context struct, ToolTags [ ]string

Did not go through proposal process but discussed with @jayconrod and @rsc as part of fixing regabi experiments. OK. - @rsc

go/parser

https://golang.org/cl/306149

  • pkg go/parser, const SkipObjectResolution = 64
  • pkg go/parser, const SkipObjectResolution Mode

OK: accepted in proposal #45104

io/fs

https://golang.org/cl/293649

  • pkg io/fs, func FileInfoToDirEntry(FileInfo) DirEntry

OK: accepted in proposal #42387

math

https://golang.org/cl/247058

  • pkg math, const MaxInt = 9223372036854775807
  • pkg math, const MaxInt ideal-int
  • pkg math, const MaxUint = 18446744073709551615
  • pkg math, const MaxUint ideal-int
  • pkg math, const MinInt = -9223372036854775808
  • pkg math, const MinInt ideal-int

OK: accepted in proposal #28538

https://golang.org/cl/315969

  • pkg math, const MaxFloat64 = 1.79769e+308 // 179769313486231570814527423731704356798070567525844996598917476803157260780028538760589558632766878171540458953514382464234321326889464182768467546703537516986049910576551282076245490090389328944075868508455133942304583236903222948165808559332123348274797826204144723168738177180919299881250404026184124858368
  • pkg math, const SmallestNonzeroFloat32 = 1.4013e-45 // 1/713623846352979940529142984724747568191373312
  • pkg math, const SmallestNonzeroFloat64 = 4.94066e-324 // 1/202402253307310618352495346718917307049556649764142118356901358027430339567995346891960383701437124495187077864316811911389808737385793476867013399940738509921517424276566361364466907742093216341239767678472745068562007483424692698618103355649159556340810056512358769552333414615230502532186327508646006263307707741093494784

OK: bug fix (more precise values), per #44058. - @rsc

net

https://golang.org/cl/307030

  • pkg net, method (*ParseError) Temporary() bool
  • pkg net, method (*ParseError) Timeout() bool

OK: bug fix (package net error must implement Error), per #45357. - @rsc

https://golang.org/cl/272668

  • pkg net, method (IP) IsPrivate() bool

OK: accepted in proposal #29146.

net/http

https://golang.org/cl/326309

  • pkg net/http, func AllowQuerySemicolons(Handler) Handler

OK: accepted in proposal #45973.

net/url

https://golang.org/cl/314850

  • pkg net/url, method (Values) Has(string) bool

OK: accepted in proposal #45100.

reflect

https://golang.org/cl/281233

  • pkg reflect, func VisibleFields(Type) [ ]StructField

OK: accepted in proposal #42782.

https://golang.org/cl/266197

  • pkg reflect, method (Method) IsExported() bool
  • pkg reflect, method (StructField) IsExported() bool

OK: accepted in proposal #41563.

runtime/cgo

https://golang.org/cl/295369

  • pkg runtime/cgo (darwin-amd64-cgo), func NewHandle(interface{}) Handle
  • pkg runtime/cgo (darwin-amd64-cgo), method (Handle) Delete()
  • pkg runtime/cgo (darwin-amd64-cgo), method (Handle) Value() interface{}
  • pkg runtime/cgo (darwin-amd64-cgo), type Handle uintptr
  • ...

OK: accepted in proposal #37033.

strconv

https://golang.org/cl/314775

  • pkg strconv, func QuotedPrefix(string) (string, error)

OK: accepted in proposal #45033.

sync/atomic

https://golang.org/cl/241678

  • pkg sync/atomic, method (*Value) CompareAndSwap(interface{}, interface{}) bool
  • pkg sync/atomic, method (*Value) Swap(interface{}) interface{}

OK: accepted in proposal #39351.

syscall

https://golang.org/cl/315281

  • pkg syscall (netbsd-386), const SYS_WAIT6 = 481
  • pkg syscall (netbsd-386), const SYS_WAIT6 ideal-int
  • pkg syscall (netbsd-386), const WEXITED = 32
  • pkg syscall (netbsd-386), const WEXITED ideal-int
  • ...

OK: new constant needed for bug fix in package os. - @rsc

https://golang.org/cl/311570

  • pkg syscall (openbsd-386), const MSG_CMSG_CLOEXEC = 2048
  • pkg syscall (openbsd-386), const MSG_CMSG_CLOEXEC ideal-int
  • pkg syscall (openbsd-386-cgo), const MSG_CMSG_CLOEXEC = 2048
  • pkg syscall (openbsd-386-cgo), const MSG_CMSG_CLOEXEC ideal-int
  • pkg syscall (openbsd-amd64), const MSG_CMSG_CLOEXEC = 2048
  • pkg syscall (openbsd-amd64), const MSG_CMSG_CLOEXEC ideal-int
  • pkg syscall (openbsd-amd64-cgo), const MSG_CMSG_CLOEXEC = 2048
  • pkg syscall (openbsd-amd64-cgo), const MSG_CMSG_CLOEXEC ideal-int

OK: new constant needed for bug fix in package os. - @rsc

https://golang.org/cl/288298

  • pkg syscall (windows-386), type SysProcAttr struct, AdditionalInheritedHandles [ ]Handle
  • pkg syscall (windows-386), type SysProcAttr struct, ParentProcess Handle
  • pkg syscall (windows-amd64), type SysProcAttr struct, AdditionalInheritedHandles [ ]Handle
  • pkg syscall (windows-amd64), type SysProcAttr struct, ParentProcess Handle

OK: accepted in proposal #44011.

testing

https://golang.org/cl/260577

  • pkg testing, method (*B) Setenv(string, string)
  • pkg testing, method (*T) Setenv(string, string)

OK: accepted in proposal #41260.
BUT it looks like we forgot to add it to the type TB interface.

https://golang.org/cl/301493, https://golang.org/cl/317269

  • pkg text/template/parse, const SkipFuncCheck = 2
  • pkg text/template/parse, const SkipFuncCheck Mode

OK: accepted in proposal #38627.

time

https://golang.org/cl/320252

  • pkg time, const Layout = "01/02 03:04:05PM '06 -0700"
  • pkg time, const Layout ideal-string

OK: not in proposal but added for commentary purposes by @robpike after discussion with @rsc. -@rsc

https://golang.org/cl/293349

  • pkg time, func UnixMicro(int64) Time
  • pkg time, func UnixMilli(int64) Time
  • pkg time, method (Time) UnixMicro() int64
  • pkg time, method (Time) UnixMilli() int64

OK: accepted as proposal #44196.

https://golang.org/cl/264077

  • pkg time, method (*Time) IsDST() bool

OK: accepted as proposal #42102.
BUT the receiver should be a value not a pointer.

https://golang.org/cl/267017

  • pkg time, method (Time) GoString() string

OK: accepted as proposal #39034.

cc @rsc @ianlancetaylor
@rsc
Copy link
Contributor

rsc commented Jun 10, 2021

I went through all of these. They are all fine BUT there are two mistakes that need correcting (marked above with BUT).

@gopherbot
Copy link

Change https://golang.org/cl/326789 mentions this issue: time: fix receiver for Time.IsDST method

@rsc
Copy link
Contributor

rsc commented Jun 10, 2021

I've sent CLs for both of the BUTs.

@gopherbot
Copy link

Change https://golang.org/cl/326790 mentions this issue: testing: add TB.Setenv

@findleyr
Copy link
Contributor

Not sure if this matters, but the go/parser proposal for SkipObjectResolution is actually #46485, which is marked Likely Accept but not yet accepted. Per discussion we're OK either documenting it or removing it pending the final outcome of that proposal.

@dmitshur dmitshur added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jun 10, 2021
@dmitshur dmitshur added this to the Go1.17 milestone Jun 10, 2021
@ianlancetaylor
Copy link
Contributor

We are missing release notes for go/parser.SkipObjectResolution (proposal still pending) and net/http.AllowQuerySemicolons.

gopherbot pushed a commit that referenced this issue Jun 17, 2021
@rsc
time: fix receiver for Time.IsDST method
b0355a3 
Only methods that modify the time take pointer receivers;
IsDST does not modify it and therefore should not.

For #42102 and #46688.

Change-Id: I4721ef7f4d7572236ae6e4d99a459b9ffb11999e
Reviewed-on: https://go-review.googlesource.com/c/go/+/326789
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@dmitshur
Copy link
Contributor

dmitshur commented Jun 28, 2021 •
edited by rsc

parser.SkipObjectResolution was documented in release notes in 1de3329, and http.AllowQuerySemicolons in 85a2e24.

As of now, there have been 2 updates to api/go1.17.txt since it was created (and this issue was based on):

  • In CL 326789, receiver for Time.IsDST method was changed from pointer to value, which was done intentionally by Russ for this issue (see comment above).

  • In CL 311129, a new RGBA64Image interface was added to image and image/draw packages. Relevant concrete types in image package were updated to implement the new interface. This corresponds to the accepted proposal image, image/draw: add interfaces for using RGBA64 directly #44808.

Remaining Work

  • Perhaps the image API changes are worth taking another look over, to confirm the implemented API matches the proposal (or deviates in ways that are intended and okay). I compared it in this comment.

  • CL 326790 has been reviewed with a minor comment, and just needs to be submitted. (CC @rsc.)

After that's done, there doesn't seem to be anything more left to audit for 1.17, so I think we can close the issue (but reopen if there are any more API changes that come up and need more work).

@dmitshur dmitshur mentioned this issue Jun 28, 2021
Closed
@rsc
Copy link
Contributor

rsc commented Jul 1, 2021

I checked the box for the image API changes.

@rsc
Copy link
Contributor

rsc commented Jul 1, 2021

Submitted the testing CL as well.

gopherbot pushed a commit that referenced this issue Jul 1, 2021
@rsc
testing: add TB.Setenv
877688c 
For #41260 and #46688.

Change-Id: I6f42742cc3234a90003136ae8798a6b0e1291788
Reviewed-on: https://go-review.googlesource.com/c/go/+/326790
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@dmitshur
Copy link
Contributor

dmitshur commented Jul 2, 2021

Thanks Russ!

Closing as done, to be reopened in the unlikely case there are new API additions that need to be reviewed for the eventual final release.

@dmitshur dmitshur closed this as completed Jul 2, 2021
@nishanths
Copy link

Not sure if this is important, but the heading text/template is missing in the issue description. The text/template changes now appear as though under the testing heading.

@nishanths
Copy link

Closing as done, to be reopened in the unlikely case there are new API additions that need to be reviewed for the eventual final release.

In case it's missed, reflect.Value.CanConvert (#47210) will be new API if a CL is merged.

@ianlancetaylor
Copy link
Contributor

Not sure if this is important, but the heading text/template is missing in the issue description. The text/template changes now appear as though under the testing heading.

Thanks, this is fine although the heading is missing.

In case it's missed, reflect.Value.CanConvert (#47210) will be new API if a CL is merged.

Thanks, the change that adds CanConvert includes a change to the API audit file (https://golang.org/cl/334669).

@golang golang locked and limited conversation to collaborators Jul 16, 2022
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Jun 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Milestone
Go1.17
Development

No branches or pull requests
7 participants
@rsc @dmitshur @ianlancetaylor @nishanths @gopherbot @heschi @findleyr